Null bytes in url could cause some problems

[maple3142]

Version

v16.6.0

Platform

Linux MAPLE 5.10.16.3-microsoft-standard-WSL2 #1 SMP Fri Apr 2 22:23:49 UTC 2021 x86_64 GNU/Linux

Subsystem

url

What steps will reproduce the bug?

There are two bugs about null byte:

const url = require('url')
const u = url.parse('http://[127.0.0.1\0c8763]:8000/')
console.log(u.hostname) // '127.0.0.1\0c8763'

new URL('a\0b')

And the error will be:

Uncaught TypeError [ERR_INVALID_URL]: Invalid URL
    at __node_internal_captureLargerStackTrace (node:internal/errors:464:5)
    at new NodeError (node:internal/errors:371:5)
    at onParseError (node:internal/url:536:9)
    at new URL (node:internal/url:612:5) {
  input: 'a',
  code: 'ERR_INVALID_URL'

The error input is apprently truncated by the null byte.

How often does it reproduce? Is there a required condition?

I think this could only happen when attacker is trying to bypass some SSRF filter in some scenario, but I think it is almost unlikely to happen in realworld.

const url = require('url')
const http = require('http')

const u = url.parse('http://[127.0.0.1\0.github.io]:8000/')
console.log(u)

if (!u.hostname.endsWith('.github.io')) {
	console.log('Sorry, you can only fetch *.github.io')
	process.exit(1)
}

http.request(
	{
		host: u.hostname, // null byte truncated
		port: u.port,
		path: u.path,
		headers: {
			Host: 'xx' // http will automatically set host header by default, and \0 will cause an error in header
		}
	},
	msg => {
		msg.on('data', data => {
			console.log(data.toString())
		})
	}
)
	.on('error', console.error)
	.end()

What is the expected behavior?

It should be invalid url, and http module shouldn't accept null byte.

What do you see instead?

Parsed successfully into a hostname with null byte.

Additional information

No response

[RaisinTen]

url.parse() has a legacy status: https://nodejs.org/api/url.html#url_url_parse_urlstring_parsequerystring_slashesdenotehost
Why not just use the WHATWG URL API? It already classifies both urls as invalid ones.

[targos]

@nodejs/url

[Trott]

The second bug described here, where new URL('a\0b') reports the input as 'a' in the error message, originates in the C++ code. The null character gets treated as the end-of-string marker, but only in the error path. It gets encoded and handled correctly, for example, in new URL('http://example.com/a\0b'). (Safari and Firefox also handle that last one correctly, but Chrome throws. I believe Chrome is deviating from the spec, but even if I'm wrong about that, for the purposes of this conversation, it doesn't matter.)

Possible solutions:

1. Use std::string instead of char * and then be very careful so we can preserve the NULL. (I suppose this approach might not work depending on the nature of how the string is handled elsewhere in the C++ code.)
2. Don't report the input string that caused the error back to the user. (Chrome and Safari both take this route.)
3. Don't worry about it. Report the truncated string. (Firefox takes this approach.)
4. Preserve the input in JavaScript and re-use it when an error is thrown from C++.

[TimothyGu]

Chrome's behavior is indeed counter to the spec. https://crbug.com/1099721

[Trott]

4. Preserve the input in JavaScript and re-use it when an error is thrown from C++.

This is the approach used in #42263.

[Trott]

4. Preserve the input in JavaScript and re-use it when an error is thrown from C++.

This is the approach used in #42263.

The WHATWG URL error message issue has been fixed. The legacy url.parse() has several possible solutions. I think the best is to bail on parsing when there is a NULL character (and possibly other C0 characters) in the host (and possibly a few other places?) and return an object with only path/pathname/href values set to anything other than null.

[Trott]

The WHATWG URL error message issue has been fixed. The legacy url.parse() has several possible solutions. I think the best is to bail on parsing when there is a NULL character (and possibly other C0 characters) in the host (and possibly a few other places?) and return an object with only path/pathname/href values set to anything other than null.

https://url.spec.whatwg.org/#host-miscellaneous

A forbidden host code point is U+0000 NULL, U+0009 TAB, U+000A LF, U+000D CR, U+0020 SPACE, U+0023 (#), U+002F (/), U+003A (:), U+003C (<), U+003E (>), U+003F (?), U+0040 (@), U+005B ([), U+005C (\), U+005D (]), U+005E (^), or U+007C (|).

A forbidden domain code point is a forbidden host code point, a C0 control, U+0025 (%), or U+007F DELETE.

[Trott]

Looks like forbiddenHostChars in url.js omits NULL even though it includes tab, lf, cr, space, etc. from the list above. It also skips checking on IPv6 hostnames. These seems like bugs that should be fixed. PR coming soon....

[Trott]

#42313