The unexpected Google wide domain check bypass
bugs.xdavidhu.me
bugs.xdavidhu.me
- At several points, unusual things were done "for some reason", following an intuition.
- The one character that does the trick was discovered by brute-forcing the browser's URL parsing logic.
- After finding that this little trick works, other related Google apps were explored (for "some reason") to see if the trick is generally applicable, which indeed it was.
- Deep diving into Chromium source to confirm why the trick works.
- How nicely the ends are tied up at the end, seeing the result of his work have an effect deep in a Closure library commonly used at Google.
---
Loved this part (my emphasis):
> I was always “afraid” to dig this deep, and would never have thought about looking into the source code of a browser, but after browsing around a bit, you realise that this code is also just code, and you can understand how it works. This is super and interesting and can be really helpful in many situations.
Edit 3: tl;dr: [/?#\] in browsers (including backslash) ends the host/authority but the RFC only specifies [/?#] as ending characters, so for example https://xdavidhu.me\.corp.google.com/blah is recognised as host `xdavidhu.me` by the browser, and host `xdavidhu.me\.corp.google.com` by the RFC - so host name whitelisting or matching can be borked for security purposes.
Edit: maybe do a search of github for this Regexp, and see what significant finds there are. Certainly bad actors are already doing so...
Edit 2: presumably there will be a gold-rush to find other bug bounties for the same underlying issue in other software or services!
Also, Google’s $6000 payout seems extremely disappointing: Google should payout on the total impact of the bug that they measure (not just the one place he happened to find). Why should the reporter have to investigate the total impact of the bug on Google in order to get a fair payout?
The “fix” made by Google to Closure is weird - because it now breaks the RFC to match the broken browser behaviour... Seems like asking for trouble (although a proper generic fix is hard). Throwing an exception on backslash seems dirty, but possibly safer...
My understanding is that this is what Google's bug bounty normally does. If for example, this was trivially extentable to access to a production user, that would be the payout.
This often becomes a pain in the ass when a single piece of software is strict on the rules that everything else ignores. Officially subdomains can't have underscores in them but the browser and everything else will allow it just fine except when you try to get a certificate and lets encrypt tells you that you have an invalid domain.
The real culprit is the domain registrar and the DNS server software. Neither of those should be enabling invalid domains to be accessible.
They already don't. DNS does not forbid underscores, and places hardly any restrictions on labels.
Hostnames do forbid underscores, and that muddies the water quite a bit. While disallowing A-records on labels with undersores would make sense, a CNAME on a label with an underscore can point to a domain without one, and the one it points to may or may not have an A-record.
It is really up to the client to decide that a domain name is invalid for their use case, but clients don't want to disallow something that appears to otherwise work fine. And so we end up in a situation where none of the specifications are truly followed.
* http://jdebp.uk./FGA/qmail-myths-dispelled.html#MythAboutBar...
Agree. Considering that they would probably make this money under a microsecond they could pay a generous reward. The impact is large though the bug is present in a single line of code.
And what is that total impact?
“I found this regex in the Google Cloud Console’s JS, Google Actions Console’s JS, in YouTube Studio, in myaccount.google.com (!) and even in some Google Android Apps. A day later I even found this line in the Google Corp Login Page (login.corp.google.com)”.
And apparently they have judged the impact less than folks who think $6000 is too small.
God help the poor engineer here. Let's say he or she (reasonably!) looked up the grammar for a URI in RFC 3986[1], the standard that defines them. Well, according to that grammar, the browser is wrong: backslash isn't valid in that position, and the exploiting string isn't a valid production of the URI grammar.
But of course, the browser has its own standard — the WHATWG URL spec — which doesn't even define a grammar for a URL, but instead defines a program in pseudocode to parse them. Normally, \ isn't valid after an authority … unless the scheme is http or https or one of several other Chosen One schemes.[2]
Uuugh. We've wrought this upon ourselves.
Also included in this test suite would be the wild and wonderful things you can type into a browser's URL bar that result in malformed HTTP requests hitting the wire… AFAIK, WHATWG doesn't yet have their own copy of HTTP…
[1]: https://tools.ietf.org/html/rfc3986#section-3
[2]: https://url.spec.whatwg.org/#hostname-state
Maybe one should argue that regex should not have accepted the URI, since it's isn't valid according to the RFC.
There are a lot of weird cases to consider. For example, how many tools correctly handle character encoding in HTTP headers?
I love WHATWG specs with their state machines. They’re all about defining exact behaviour based upon what has already been implemented by extant software, and define comprehensively how every possible input should be parsed. They’re easy to transcribe in code without error. These things get you compatibility, and avoid security bugs where people have implemented different behaviour in different places—as was the case in this instance. As a parser spec, the HTML spec is my favourite, because it makes implementing something that is fiendishly complex genuinely easy.
I would love something equivalent for HTTP/1 and MIME messages.
Of course it's not as though Google had a choice at the outset, I'm sure some non-trivial number of web sites magically didn't work (and maybe still don't work) if you insist that https://www.example.com\works\fine\on\my\pc doesn't do what the person writing it expects... I think we can blame some early Internet Explorer build for first having this work (I can't see Netscape allowing it) and then it becomes a choice whether to be a purist or sigh and move on once IE gets popular and real people do it.
[edit] Apparently CP/M took it from VMS, which used period as a path separator.
I had a CP/M machine in the 70's, and I remember it quite differently.
My memory was that CP/M used normal slashes for directory separation, and the backslash was a Microsoft bodge so it could pretend it didn't have Seattle Computer Products copy CP/M 80 to make DOS.
I'm not at home, so I can't verify the slash situation on any of my CP/M emulators. It's also possible that I am not remembering correctly because I was also using Vaxen and PR1MEOS in those years.
Besides CPM filesystem wasn’t hierarchical.
This stack-exchange post[1]
Looks like that's a corruption of the information from a MS blog[2], which does not mention CP/M but DOS 1.0 as taking it from DEC in general, including systems older than VMS.
> Besides CPM filesystem wasn’t hierarchical.
Which I mentioned in my comment.
1: https://superuser.com/questions/176388/why-does-windows-use-...
2: https://web.archive.org/web/20120304135002/http://blogs.msdn...
E.g. tail recursive strchr:
const char *strchr_rec(const char *str, int ch)
{
if (*str == 0)
return NULL;
if (*str == ch)
return str;
else return strchr_rec(str + 1, ch);
}
const char *strrchr_tail(const char *str,
int ch,
const char *seen)
{
if (*str == 0)
return seen;
if (*str == ch)
return strchr_tail(str + 1, str, ch);
return strchr_tail(str + 1, seen, ch);
}
const char *strchr_rec(const char *str, int ch)
{
return strchr_tail(str, ch, 0);
}
The only problem is that NUL-terminated strings are so darn easy to program with, everyone who comes into contact with them wants their share of the action, instead of using someone else's well-tested code.
As we speak, someone is likely writing yet another loop that builds up a NUL-terminated string directly, and will forget to write the final NUL in some case, or go past the end of the buffer in another.
Want to break a string into tokens on delimiters? Just overwrite the delimiters with null bytes and you're done. (Oops, the caller didn't expect their string mutated. Worse, it was a literal ...).
If you make programmers maintain a length field followed by data, or some such thing, they will quickly go off scampering for library functions to everything.
What we should never do is compare unencapsulated NUL-terminated string manipulation with the manipulation of another representation using a robust, debugged library.
Compare open-coded to open-coded, wrapped to wrapped.
I much prefer the approach used by languages like Rust, where &str and &[T] are (pointer, length) rather than just a pointer.
Usage can be identical to your classic NUL-terminated string, depending on the language. (It is in Rust, with CStr.)
There is only one potential disadvantage: pointers use twice as much memory; but in practice, this disadvantage is entirely theoretical.
But there are several serious advantages: it lets you represent zero values (U+0000 for &str, integer zero for &[u8], &c.), take slices that aren’t suffixes (reducing either code complexity or memory usage, sometimes drastically), calculate the string length in constant time (and that’s regularly a big deal), and avoid a pitfall that has been the cause of many critical security vulnerabilities (buffer overrun).
Want to break a string into tokens on delimiters? Easy, regardless of the length of the delimiter (it could be zero, two, seventeen—while with NUL-terminated strings it had to be one), without needing to worry about whether it’s safe to mutate the string.
"Such and such a data structure ... in the middle of Rust" is not a discussion about string representation per se any more.
Rust must have null terminated strings at some level, because it has to integrate with platforms where null terminated strings are the API currency for text. Yet, Rust programs don't fall down because of null-terminated strings.
Likewise, if you have a pointer and length, but no language or library support for it, just a "high level assembly language", you will screw it up.
The pointer has to point to something. There will be cases where that something is a fixed buffer on the stack or in a dynamic object. Out of a hundred programmers coding their pointer and length with fixed buffers, a good many will cause overflows, and a good many will write memory-safe code that causes consequences due to silent truncation.
In C, we can cause problems like:
* returning a pointer to a stack-allocated { length, ptr } structure, which turns to garbage.
* leak dynamically-allocated { length, ptr } objects, or use them after freeing.
* making copies of a { length, ptr } structure (e.g. pass by value), and not keeping all of them in sync when ptr and/or length are updated.
* sticking a literal "string" into ptr, but then treating it as dynamic elsewhere and trying to free it
* sticking a literal "string" into ptr, and then somewhere trying to modify a character.
No, that's not what they are saying.
They're just saying that to blame the representation for user mistakes and then present an alternative representation where user mistakes are prevented by library functions is comparing apples to oranges.
IMO: if it’s not a hobby project just use a library for slicing and manipulating strings. Really you should think twice before doing it at all.
I feel like I hardly understand the idea, because a / and \ is just as distinct as a b and a p. We don't make mnemonics for those?
Can confirm. Just yesterday on NPR.
The average person does know (or at least should know) the difference between an exclamation mark and a question mark; it's part of literacy. One could argue that knowing the difference between a forward slash and a backslash is part of digital literacy (and that it's a worthy goal for the average person to have digital literacy).
In any event, blaming Windows for this is a bit silly.
new URL("https://user:pass@xdavidhu.me\\test.corp.google.com:8080/path/to/something?param=value#hash")Thanks for the write-up.
https://www.google.com/about/appsecurity/reward-program/inde...
I guess they were awarded $6K instead of the full because they didn't demonstrate it against more substantial properties. But either way the ceiling is quite low on these classes of vulnerabilities.
I'm curious to know, but I'd think that the author would've tried to also exploit it in those other apps, and the fact that it's not documented here may be because they didn't find any issues with the other ones? It's a pretty generic regex that has many uses, and may be used for other stuff in other places.
(The work is important! But people who find web bugs like this tend to make their money on volume, not on a couple major hits a year, like the RCE researchers do.)
No they’re not. Just because this happened to be a URL whitelist bug doesn’t mean that this is the primary work that the researcher who found the bug does. Did a URL whitelist validation researcher find this, or did someone researching user auth access find this bug? If Google knew exactly who they wanted to find this bug, then Google would have already found it themselves.
The black hats. $6,000 from Google, or $600,000 from some shadowy company in UTC+02. Google makes these choices too easy.
Who Google and other service providers are competing against are grayweb exploit dealers. That's the real market, and every popular site, api, and program is subject, regardless if they're on H1 or not.
Sure there's some whitehat types who'd only tell the companies directly. But the rest.... Well.
And again, it's not a status thing. You can probably make quite a lot of money on bugs like this; you simply find a lot more of them than anyone generates RCE exploits.
Seriously though, nice find! It's often surprising how tiny little curiosities can lead down the rabbit hole to such interesting problems. In this case, it was a fairly logical flawed assumption, but the mess of regex made it just hidden enough to be tough to spot.
Too soon.
If a backslash can end a security origin, I am going to guess there are a lot of other vulnerable bits of code...
One more reason joining and splitting strings with security impact is a bad idea.
Doing it in a standard/shared library is more secure than bespoke code on each page, since at the very least as we witness here it can all be updated together as needed. You can add additional abstraction, but someone eventually has to do this work.
Even web browsers themselves aren't immune from bugs in "joining and splitting strings with security impact" for URL comprehension. All you can really do is try not to re-write known secure code.
Plus this buggy RegX was from the standard on URL parsing:
What a great find.
It highlights individual parts of the regex and provides human explanation, e.g. https://regex101.com/r/xKG7pE/1
But I guess Closure dates from the IE6 era where trusting browsers for correctness was probably not a good idea..
It's the URL object (https://developer.mozilla.org/en-US/docs/Web/API/URL), isn't it? We tried using it at work... and found out that, as always seems to be the case, it's available everywhere except on Internet Explorer.
blacklist: [^/?#]+
whitelist: [a-z](?:[0-9a-z]|-(?!(?:$|\.)))*(?:\.[a-z](?:[0-9a-z]|-(?!(?:$|\.)))*)*I’m just trying to point out that securing via Regexps is never simple, even for the very skilled. If someone posts a comment with a Regexp, it is often broken and one can usually find problems with it.
Edit: and what are the $ in there for? They look redundant.
$ are there because you can't have '-' at the end of the label. You'd need more chars there if you want to use this as a part of full URL parser. Or just use browser's facilities for parsing URLs.
Also noone cares if you can DoS your own browser tab/iframe. You probably can't anyway, because browser has other mechanisms to prevent that. But yes, you'd need to limit this too in real code, to the max length of the label part of the domain name.
Because fuck trying to explain how that code actually works to anybody else, or myself a year from now.
The regex approach, btw, is even worse. Parse your URLs before doing things with them.
That's what makes this crazy. Generally I agree that people shouldn't abuse regular expressions for parsing security-critical data. But it just so happens that a standards-compliant URL can be trivially parsed using a naive regular expression that relies on the semantics of the gen-delimiters character set in the standard. In fact, it's really hard to break such a naive regular expression even when we move beyond the domain of standards compliance. One glaring exception is anything that introduces ambiguity between the userinfo authority section and the path, which the browsers have chosen to do. sigh
A new RFC should be issued that updates the syntax to include the semantics which browsers give to backslash. Among other things, this will help to publicize the issue.
Calculating file origin: "Unfortunate as it is, this is left as an exercise to the reader.”
3.7. Host equivalence: “To determine whether a host A equals B, return true if A is B, and false otherwise.”
“The application/x-www-form-urlencoded format is in many ways an aberrant monstrosity”
Though you have to wonder, if there wasn't a bounty...
Good idea - I know this would work on some places. I never thought of it like this.
My experience is regex seems like line noise to people. If you show them the multi line commented form they get it, and can then appreciate the one liners.
I’ve thought a neat IDE trick would be doing that expansion, into this visualization followed by multi line text you edit, and a column to paste sample text into and see matching / back-refs / captures happen. Stays one liner in code.
Standalone tools do it, slicker if inline.