git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] url: do not allow %00 to represent NULL in URLs

From
brian m. carlson <sandals@crustytoothpaste.net>
Date
Jun 4, 2019, 01:02 UTC
Message-ID
<20190604010243.GR8616@genre.crustytoothpaste.net>
In-Reply-To
<20190603204526.7723-3-matvore@google.com>
On 2019-06-03 at 20:45:26, Matthew DeVore wrote:
Show 5 quoted lines
> There is no reason to allow %00 to terminate a string, so do not allow it.
> Otherwise, we end up returning arbitrary content in the string (that which is
> after the %00) which is effectively hidden from callers and can escape sanity
> checks and validation, and possible be used in tandem with a security
> vulnerability to introduce a payload.

So I think the reason you've stated is good and I agree that we shouldn't decode data we're not going to use. However, I'm also interested in the cases in which we decode data and don't want to allow NULs, because we should, in general, allow bizarre URLs as long as they're URL-encoded.

It looks like several of the places we do this are in the credential manager code, and I think I can agree that usernames and passwords should not contain NUL characters (for Basic auth, RFC 7617 prohibits it). It also seems that the credential code decodes the path parameter before passing it on, which is unfortunate, but can't be changed for backward compatibility reasons.

And then the other instances are a file: URL in remote-testsvn.c and query parameters that have no reason to contain NULs in http-backend.c.

So I think overall this is fine, although we probably want to change the commit summary to say "NUL" instead of "NULL".

-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
Previous: Matthew DeVoreNext: Matthew DeVore
Message 3 of 9 in “Harden url.c URL-decoding logic”
  1. 0/2 Harden url.c URL-decoding logicMatthew DeVore, Jun 3, 2019
  2. 2/2 url: do not allow %00 to represent NULL in URLsMatthew DeVore, Jun 3, 2019
  3. brian m. carlsonJun 4, 2019
  4. Matthew DeVoreJun 4, 2019
  5. René ScharfeJun 4, 2019
  6. Matthew DeVoreJun 4, 2019
  7. 1/2 url: do not read past end of bufferMatthew DeVore, Jun 3, 2019
  8. René ScharfeJun 4, 2019
  9. Matthew DeVoreJun 4, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.