Hacking GitHub's Auth with Unicode's Turkish Dotless 'I' • Hacker News Board

Re: Hacking GitHub's Auth with Unicode's Turkish Dotless 'I'

#171

Post by BoppreH » Tue, Dec 17, 2019, 1:27 PM UTC

founderling wrote: post #138

BoppreH wrote: post #36

I love Unicode, but I'm more and more coming to the conclusion that strings are evil and should be treated as opaque byte arrays, whose only available operation is rendering into a bounded area. I now see any other string operation as code smell. It's scary how much of our infrastructure relies on strings, given how few guarantees string operations actually give. Take files names, for example. Two visually identical…

opaque byte arrays, only available operation is rendering into a bounded area Goodbye web then. Because the whole request you get from a client is nothing but strings. So when you run an onlineshop and a customer orders "7" screwdrivers - then you are screwed. Because what does an "opaque byte array" of "opaque byte arrays" cost? How much shipping will that be? But at least you have a brand new customer: Henry@gmail.…

That's a good point, these problems are not easy to solve. But they should be solved.

For example, the user types "7" into the UI control. The UI control knows it's supposed to hold a number, so it has methods that return the parsed number (done by the OS, very carefully), or an error to the user.

Similarly, you don't send the product name, but the ID of the product the user selected from their fuzzy search. I would go as far as using this technique for the email too.

Re: Hacking GitHub's Auth with Unicode's Turkish Dotless 'I'

#172

Post by thaumasiotes » Tue, Dec 17, 2019, 1:30 PM UTC

2T1Qka0rEiPr wrote: post #163

thaumasiotes wrote: post #149

Earlier quoted context omitted.

I want to note a separate issue of defensive coding that comes up in the writeup: > GitHub's forgot password feature could be compromised because the system lowercased the provided email address and compared it to the email address stored in the user database. If there was a match, GitHub would send the reset password link to the email address provided by the attacker The logical flow is: 1. Get the email address fro…

I'm not sure I fully understand. What do you mean by step 2? If I entered "myemaıl@example.com" into the reset field, are you saying that step 2 would be the process of doing some normalization to try to find a matching account? If I reset a password, don't I only provide an email address by means of doing so? Therefore, doesn't the service merely attempt to match an email to an existing account within the DB? I beli…

I reply separately to observe that the flow you describe is bugged in a more obvious way: if you ask only for an email address, and then discover the related account by normalizing that address before doing a database lookup, it's a serious error to then send the reset email (which controls an account you looked up using the normalized address) to the original address. You found the account by looking up a normalized address; the original address isn't even known to be associated with the account.

In that case, there are three options:

1. Send the reset email to the address you pulled from the database. (correct)

2. Send the reset email to the normalized attacker-provided address. (wrong but "probably fine"; this is the bug I was talking about in the first place)

3. Send the reset email to the original, non-normalized attacker-provided address. (wrong and definitely a problem)

Re: Hacking GitHub's Auth with Unicode's Turkish Dotless 'I'

#173

Post by mathewsanders » Tue, Dec 17, 2019, 1:33 PM UTC

thaumasiotes wrote: post #157

gingerlime wrote: post #153

Earlier quoted context omitted.

Yes, you're absolutely right. The real bug was sending to the "wrong" matching email. But this is what makes this bug so hard to find. You're looking at "equal" strings, so why should it make a difference if you pick A or A if A === A ? :)

Hard to find, but it's the kind of thing that will hopefully come up in code review. Consider this pseudocode with helpful pseudo-hungarian notation: username = request.post_params('username') evil_email = request.post_params('email_address') user = get_user_by_name(username) good_email = user.email_address if good_email != evil_email: # Hackers! else: reset_password(user.id, evil_email) # it's fine; it's the same as…

I’d never heard of Hungarian notation, but searching for info I realize that I’ve seen this pattern so often but never knew it had a name :) TIL https://en.m.wikipedia.org/wiki/Hungarian_notation

Re: Hacking GitHub's Auth with Unicode's Turkish Dotless 'I'

#174

Post by pjc50 » Tue, Dec 17, 2019, 1:43 PM UTC

thaumasiotes wrote: post #149

BoppreH wrote: post #36

I love Unicode, but I'm more and more coming to the conclusion that strings are evil and should be treated as opaque byte arrays, whose only available operation is rendering into a bounded area. I now see any other string operation as code smell. It's scary how much of our infrastructure relies on strings, given how few guarantees string operations actually give. Take files names, for example. Two visually identical…

I want to note a separate issue of defensive coding that comes up in the writeup: > GitHub's forgot password feature could be compromised because the system lowercased the provided email address and compared it to the email address stored in the user database. If there was a match, GitHub would send the reset password link to the email address provided by the attacker The logical flow is: 1. Get the email address fro…

This is one of those cases where what we want is a tainting system for strings, not an encoding problem. The only language I've seen attempt this was Perl, and even then intermittently.

It should be made as difficult as possible to pass user input directly to something vulnerable like an email-sending API, without first laundering it through "validation". Unfortunately it can be very hard to do good validation, but in a tainting system you would find it easier to use the already-trusted value from the database for the user's email rather than the untrusted one from user input.

(Comparing two email addresses with toupper rather than doing a full RFC2822 comparison is another mistake, although I can see why nobody bothers to do it properly)

Re: Hacking GitHub's Auth with Unicode's Turkish Dotless 'I'

#175

Post by heydabop » Tue, Dec 17, 2019, 1:58 PM UTC

thaumasiotes wrote: post #149

BoppreH wrote: post #36

I love Unicode, but I'm more and more coming to the conclusion that strings are evil and should be treated as opaque byte arrays, whose only available operation is rendering into a bounded area. I now see any other string operation as code smell. It's scary how much of our infrastructure relies on strings, given how few guarantees string operations actually give. Take files names, for example. Two visually identical…

I want to note a separate issue of defensive coding that comes up in the writeup: > GitHub's forgot password feature could be compromised because the system lowercased the provided email address and compared it to the email address stored in the user database. If there was a match, GitHub would send the reset password link to the email address provided by the attacker The logical flow is: 1. Get the email address fro…

I've always used values I pulled from the DB even when they matched (theoretically) the value I passed into the where clause and never really had a reason why, it just felt better that way. Thanks for validating that.

Re: Hacking GitHub's Auth with Unicode's Turkish Dotless 'I'

#176

Post by eyegor » Tue, Dec 17, 2019, 2:04 PM UTC

jazzabeanie wrote: post #161

thaumasiotes wrote: post #157

Earlier quoted context omitted.

Hard to find, but it's the kind of thing that will hopefully come up in code review. Consider this pseudocode with helpful pseudo-hungarian notation: username = request.post_params('username') evil_email = request.post_params('email_address') user = get_user_by_name(username) good_email = user.email_address if good_email != evil_email: # Hackers! else: reset_password(user.id, evil_email) # it's fine; it's the same as…

If they do have multiple email addresses associated with one account, and they don't want to send a password reset to all of them, then you can see how it would happen. ``` if evil_email not in good_email_addresses: # Hackers! else: # just reset with provided email. If I thought there was a potential security issue I would have already addressed it. ``` So easy to be lazy at this point, especially under time pressure…

Off topic, you need the triple backticks on their own lines for them to render here.

Re: Hacking GitHub's Auth with Unicode's Turkish Dotless 'I'

#177

Post by squiggleblaz » Tue, Dec 17, 2019, 2:05 PM UTC

pjc50 wrote: post #174

thaumasiotes wrote: post #149

Earlier quoted context omitted.

I want to note a separate issue of defensive coding that comes up in the writeup: > GitHub's forgot password feature could be compromised because the system lowercased the provided email address and compared it to the email address stored in the user database. If there was a match, GitHub would send the reset password link to the email address provided by the attacker The logical flow is: 1. Get the email address fro…

This is one of those cases where what we want is a tainting system for strings, not an encoding problem. The only language I've seen attempt this was Perl, and even then intermittently. It should be made as difficult as possible to pass user input directly to something vulnerable like an email-sending API, without first laundering it through "validation". Unfortunately it can be very hard to do good validation, but i…

You could do that with a modern type system. For instance, you create types:

UserInputString, UnvalidatedEmailString, ValidatedEmailAddress.

At least the first two of them are type wrappers for strings - opaque to the type checker, but transparent to the runtime. Rust and Haskell have wrappers like this. Typescript does not.

Keep the internal details private to the module so that application code doesn't concern itself with the details.

The module that sends emails only provides code that accepts a ValidatedEmailAddress and sends it an email, or accepts an UnvalidatedEmailString, records it in the database, send it an email, and Validates it. Next time you load it, if it's been validated, you get a ValidatedEmailAddress - as long as it's been validated.

Sensibly, there would be no (public) code to convert the ValidatedEmailAddress to a database id, since (a) you never application code want to run "UPDATE email_address SET address = 'ketchup@tomato.sauce' WHERE id = 5", because it needs to be validated. Likewise, you never want application code to run "INSERT INTO user_email(email, user) VALUES (5, 6)". (Something like the latter will of course occur, in the code responsible for recording invalid emails and retrieving validated emails.)

Re: Hacking GitHub's Auth with Unicode's Turkish Dotless 'I'

#178

Post by jrochkind1 » Tue, Dec 17, 2019, 2:19 PM UTC

thristian wrote: post #71

dathinab wrote: post #57

Earlier quoted context omitted.

Note that Unicode did add a "uppercase-ish" ß, as it does appear in German, but only in context of an all caps word e.g on a sign board, so captilazation of a whole word to SS and that new all caps ß are both correct (not sure if UNICODE changed the capitalization rules or just added that strange all caps ß)

For backwards compatibility reasons, the capitalization rules can't be changed for existing characters. So normalizing by naive case-folding now requires at least three steps: "ẞ".to_lower() → "ß" "ß".to_upper() → "SS" "SS".to_lower() → "ss" (there's a standard for how to compare strings case-insensitively that doesn't involve repeated case-folding, but it's much more complex)

Wow, I did not know about this weird edge case, interesting, thanks!

Re: Hacking GitHub's Auth with Unicode's Turkish Dotless 'I'

#179

Post by swish_bob » Tue, Dec 17, 2019, 2:33 PM UTC

microcolonel wrote: post #28

kevin_thibedeau wrote: post #27

Earlier quoted context omitted.

Senders don't get to dictate how a recipient encodes their addresses. RFC 822: The local-part of an addr-spec in a mailbox specification (i.e., the host's name for the mailbox) is understood to be whatever the receiving mail protocol server allows.

I am well aware of that, but I'm comfortable requiring that new customers don't register an account with an email address foolishly designed to resemble another customer's email address in this particular way. We don't throw away their specified mailbox address, we just don't accept registrations which look suspiciously similar, or intended to cause confusion. I repeat, this has absolutely nothing to do with the mail…

So, when I registered for my primary email account (many, many years ago), Firstname.Lastname@provider was already taken, so I took FirstnameLastname@provider.

Are you suggesting I shouldn't be allowed an account with you if the person who beat me to my preferred email address also beat me to registering with you?

Re: Hacking GitHub's Auth with Unicode's Turkish Dotless 'I'

#180

Post by swish_bob » Tue, Dec 17, 2019, 2:38 PM UTC

RyanCavanaugh wrote: post #67

_jal wrote: post #61

Earlier quoted context omitted.

Sure, and meanwhile if there are collisions without periods on that domain (randy@somewhere, r.andy@somewhere and rand.y@somewhere, for instance), only one of them gets an account. In the absence of being able to count on specs, I guess the user should expect a race?

The fact that we just now found out that this is the case, yet would be completely unable to find anyone complaining about it except in hypothetical terms, tells you exactly how important it is.

How would they complain to you? How would they know what the problem is?

Assuming they don't just fail to register and move on ...