Hacking GitHub's Auth with Unicode's Turkish Dotless 'I'
eng.getwisdom.io
eng.getwisdom.io
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 file names may map to different files (because confusables[1]), or two different names map to the same file (because normalization[2]), or the ".jpg" at the end may not actually be the extension (because right-to-left override[3]), not to mention names with newlines or backspaces in them, and inconsistencies between operating systems.
I would go as far as blaming our overreliance on strings for all the injection attacks we see (XSS, SQL, command, etc).
[1] https://unicode.org/cldr/utility/confusables.jsp
[2] https://developer.apple.com/library/archive/qa/qa1173/_index...
[3] https://krebsonsecurity.com/2011/09/right-to-left-override-a...
Now take all that and multiply by the complexity of world languages, many which don't even map to one glyph == one morpheme. The ol' apple message crash bug was due to the property of some Arabic not being monotonic in rendering space vs string length.
I think we could have skipped utf8 and just gone to 4byte runes. But even then, that would not have avoided the above bug.
Utf16 is a hot mess though, worst of all worlds.
> Who (process-wise) is responsible for converting bytes to pixels?
The operating system, with minor exceptions (word processors, for example). Rendering logic is too complicated to be embedded into every application. And you get better accessibility and consistency.
> How do users on social media put in their name? How is it stored?
Keyboards, or their preferred input methods, and stored in UTF-8. I'm not saying to get rid of all strings, just don't use it for infrastructure.
> How do users get urls with specific usernames?
You don't, because that's how you get little Bobby FRACTION-SLASH. Also, if you have a valuable namespace like URLs, people will hack each other to get valuable names, but that's only tangentially related.
> Now take all that and multiply by the complexity of world languages, many which don't even map to one glyph == one morpheme. The ol' apple message crash bug was due to the property of some Arabic not being monotonic in rendering space vs string length.
That's exactly my point! You get this multiplied complexity when people try to peek into string contents instead of treating them like black boxes. Stop with the dangerous string operations and you now support usernames with zalgo-ed hieroglyphs if that's what users want.
> I think we could have skipped utf8 and just gone to 4byte runes. But even then, that would not have avoided the above bug.
> Utf16 is a hot mess though, worst of all worlds.
Agreed with UTF-16. I like UTF-8, and I honestly think it solved our encoding problems for non-legacy applications. Everyone should be using it, as long as the contents are for human consumption only.
It's a partial fix that makes bugs harder to catch. There's also the BOM issue, and not being ASCII compatible...
Consider things like databases (relational/document/key-value/etc.), JSON payloads, compression algorithms, battery life, etc. etc.
There is a problem with lots of developers assuming UTF-16 is fixed-width (just look at this thread), but that's a mistake for any UTF encoding, and it's almost always a mistake in any encoding--you shouldn't be indexing into strings.
People say, "just use UTF-8, you never have to worry about it and it's ASCII compatible", but you shouldn't be using the ASCII compatibility (arguably this is an anti-feature) and you shouldn't have to worry about the details like BOM because you're using your language's built-in string support or a library right?
Where there is significant markup, UTF-16 encoding is rarely more space-efficient than UTF-8. When compression is involved, there is rarely a difference.
ASCII: 1 byte
European characters: 2 bytes
Asian characters: 3 bytes
There is no reason you'd want this in Asia. China uses GB2312, which flips it around: ASCII: 1 byte
Asian characters: 2 bytes
European characters: 3 bytesNote that “Asian” is quite a gamut in terms of UTF-8 size: Chinese is at the very compact end of the spectrum and the Burmese script at the other end: https://hsivonen.fi/string-length/#counts
I'm a little skeptical about that table though, because as you point out, there's not a great way of figuring "meaning per character". For some more (real flawed) comparison, the English version of The Tale of Genji is ~60k words and 224 pages, and the Chinese version is ~75k words and 300 pages. [1] [2]
So yeah, I'm skeptical about the claim that some languages have more meaning per character, and as a result you'll end up storing less text overall. I think it'd be cool to look at more data about this though, like (for example) stats from Treasure Data [3].
But regardless, it's pretty indisputable that UTF-16 is a lot better space-wise for languages that tend to be multibyte in UTF-8. Mostly what I'm trying (Quixotically) to say is "UTF-8 the world" ignores a lot of the world.
[1]: https://www.readinglength.com/book/isbn-4805314648
Note that link [2] says "guess [of # of words] based on page count". 75k words, 300 pages is one statistic, not two separate statistics. (Estimating words based on page count is likely to be highly reliable, but still.)
> I'm skeptical about the claim that some languages have more meaning per character, and as a result you'll end up storing less text overall.
Your skepticism is unwarranted. From a pure information-theoretic perspective, the claim that some languages have more meaning per character is a slam dunk, and less than a second of examination proves it conclusively.
For example, an entire English novel is unlikely to use more than 256 unique characters. A Chinese novel couldn't use anywhere near that few without sounding incredibly artificial.
Here are some single-character words in modern Mandarin:
高 - high/tall
低 - low
大 - big
小 - small
最 - most (superlative marker)
更 - more (comparative marker)
到 - arrive
走 - leave (go away)
玩 - play (e.g. a game)
看 - look
听 - listen
贵 - expensive
爱 - love (verb)
恨 - hate (verb)
龍 - dragon
For something more representative, here are some song lyrics -- I'll enclose every word of multiple characters. Unenclosed stretches of text are words of one character each: 都怪我不[小心]地
[偷看了]你的[眼睛]
[迷失了][自己]
[不知][为什么][欢喜]
[期待]听你的[声音]
是[那么][甜蜜]
我[拿着][电话]在你家门外
想对你[说出]我[内心]中对你的[期待]
[忽然][感觉][浑身][热血][澎湃]
[今天]我[想要][[鼓起][勇气]][大声][表白]
How many distinct one-character words do you think English could practically support? How many twos? In this verse-and-a-half, there's one word that's always[1] three characters and two that have reached three characters by picking up a verb suffix, for a total of three words that are longer than two characters. (鼓起勇气 is kind of a special case, in that it's a well-known fixed expression, but its meaning is transparent as an ordinary combination of the two words 鼓起 and 勇气, which also see use outside the expression.)[1] Actually, 什么 ["what"] has the vernacular contraction 啥, and this also applies to 为什么 ["why"], so you could argue that the word is sometimes just two characters.
Yeah I mean, there's no question ideographic languages have more meaning per character than alphabetic languages. But other comparisons and considerations aren't as obvious:
- how do ideographic languages compare with each other?
- do people using ideographic languages write more?
- are ideographic languages as effective at compression as general (or special) compression algorithms?
Moving up the conceptual ladder from "average bytes per codepoint" to "average size of encoded tweet" (for example) is a big leap is all I'm saying.
That one we know; the compression algorithms are more compressive. For example, compressed Chinese text takes up less space than the same text uncompressed. Ideographic languages are still languages that real humans have to use, and they feature redundancy because that helps everyone. Compression algorithms have the luxury of stripping that redundancy out.
> how do ideographic languages compare with each other?
Of modern languages, only Chinese and Japanese could really be described as ideographic. (Japanese much more so than Chinese, in fact.) Chinese will have more meaning per character, because Japanese makes heavy use of the comparatively less meaningful kana. (Interestingly... it has to do this precisely because of its more ideographic nature.)
> do people using ideographic languages write more?
No idea.
A related question is if people using ideographic languages write electronically when they do, and I suspect yes.
Language Log has several articles on the phenomenon where Chinese seem to be forgetting how to write characters due to IT input methods. https://languagelog.ldc.upenn.edu/nll/?p=7142
Handwriting is actually a moderate-level problem for me -- I can't recognize most handwritten characters. Often a special handwriting form will be used.
OK, I lied. China doesn't use GB2312 anymore. They use its update, GB18030, which carries arbitrary Unicode data just as UTF-8 does... except that, like GB2312, it puts the Asian characters in two bytes and the European characters in three.
It's certainly not obvious to me that UTF-8 is better for globally appropriate software. It looks worse. Software for Europeans, sure.
There are more important qualities than optimizing byte length. Processing GB18030 as Unicode scalar values involves lookup tables. A single byte error cascades potentially further than in UTF-8.
If optimizing Chinese byte length is really important for you, UTF-16 is easier to work with than GB18030.
> When compression is involved, there is rarely a difference.
Rare is the situation where smaller input to a compression algorithm leads to larger output, and UTF-16 is smaller in lots of languages. It might rarely make a difference you, but that's not the same as rarely making a difference.
Most server-side applications should never have to know what these concepts even are. Or any library that is not user-facing. They get bytes from the UI layer, and they can keep them as opaque bytes. For user-facing apps, you can ask your renderer library for a pixel-width or similar for a string, and let them handle how to parse it. Very little code ever needs to know about unicode.
Any kind of input-sanitization is vastly simplified in utf8, and that makes it worth it for me. For me the really troubling trends are conventions like Rust Utf8Error, where they can cause what I'd consider a UI-related exception in code that had no business even interpreting what those bytes are. Unfortunately, every API uses strings, so they are kind of hard to avoid. It introduces what I'd consider a software layering problem.
Maybe others here with more experience with internationalization can chime in and tell me I'm wrong.
> For me the really troubling trends are conventions like Rust Utf8Error,[…]
Interesting. Isn't that only returned when the input bytes contain a non-UTF-8 byte sequence? How is Rust's approach different from other languages?
As a Rust user: this is what your function returns if the user inputs non-UTF-8 bytes into something that expects UTF-8 bytes and the programmer explicitly choses not to handle that error.
I don’t see anything wrong with that. Sometimes you might be interested in receiving, processing, storing valid UTF-8 strings rather than arbitrary byte sequences, that may or may not be able to be translated back into something valid that you can display.
I always hated to do encoding related work with a passion before I started using Rust. Rust forced me to do it the right way and actually understand why I am doing it a certain way. When it comes to encoding I feel safer in Rust then e.g. in Python, despite having used it for four times as long.
This is a common misconception with UTF encodings.
> A character doesn't mean anything in Asian languages, and any attempt to use a fixed-length encoding is pointless.
Totally agree, indexing into a string is bad practice, no matter the encoding (because you probably can't ever guarantee what the encoding will be, or how it was converted, etc.) This is true for UTF-8 and UTF-16, and really any encoding because again, you can't be sure what you're dealing with. Code points, "characters", glyphs, etc. are all concepts that work at different parts of the stack (well, characters doesn't), which again is true for all encodings.
The advantage UTF-16 has for languages that tend to be multibyte is it's representation takes up less space. Other than that, it has all the same disadvantages any other encoding has.
> Any kind of input-sanitization is vastly simplified in utf8, and that makes it worth it for me. For me the really troubling trends are conventions like Rust Utf8Error, where they can cause what I'd consider a UI-related exception in code that had no business even interpreting what those bytes are. Unfortunately, every API uses strings, so they are kind of hard to avoid. It introduces what I'd consider a software layering problem.
This is the problem right here. The "UTF-8" the world initiative ignores that UTF-16 is a lot more practical for most people, and as a result you get platforms expecting UTF-8 that really have no business doing so.
Make string ops compile to byte array ops?
how would Google work? how would a web browser work, e.g. if you type in a URL?
And what's the problem of letting the user type a URL? You take the resulting string (given to you by the OS subsystem responsible for keyboard input), and stuff that into your favorite HTTP library.
Don't get me wrong, strings are absolutely necessary. I'm not suggesting we switch to pictograms. But string processing should be treated like the dangerous operation it is, like pointer math and cryptography.
But the Github issue was sort of subtle. In hindsight it seems like an obvious one, but it's a mistake I typically see in a lot of code reviews I've done where people just reach for the convenient variable without thinking behind the intent and 'meaning' behind it. The problem is you would probably have code like this:
sendChangePassword(string email) {
userDetails = getUserDetails(email);
if (userDetails !== null)
MailerHelper.sendChangePasswordEmail(email, userDetails.name);
}
How would the don't look at strings thing apply? Well you could not use .toLowerCase inside the getUserDetails, then you are treating the email as a stream of bytes. But it would allow someone to sign up multiple times as fred@gmail.com, FRED@gmail.com. This might be bad for you (spammers) and bad for the user (they don't realise they actually have 2 accounts).The alternative could be 'well use a library written by experts for dealing with email duplication issues', i.e. detecting that string A and string B map to the same email or not. But we criticize ourselves for the NPM dependency madness, right? So some balance is needed.
It is tricky to not deal with strings in some way!
One pattern I am keen on, and will probably use in a refactor I am doing is using types to add meaning to strings. So in Typescript for example, don't pass a string, but create a wrapper class called, say EmailString, than on construction does some validation (or maybe none), but as a minimum you have documentation of what the string means. You can shift-F12 the constructor to check the hopefully few places it is constructed to make sure they do the write thing, then in the 100's of consuming places you know you are dealing with an email.
I put abuse detection system in the same category as tests, it gets a free pass on most cleanliness requirements.
The typed strings pattern is a great one. In this case you could go as far as having a RegisteredEmail types, and requiring that in your MailerHelper.
I just ranted about why ASCII is important to programmers: https://news.ycombinator.com/item?id=21760540
and this is a perfect example. ASCII has almost a 1-to-1 mapping between screen representation and byte representation. Once you know your font will differentiate between 1 i L | l and 0 O you still need to know a bit about control characters and you are good to go.
Unicode has tons of pages, control characters, diacritics and rendering oddities that make it hard to use like a tool.
I think your approach of considering like bytes to render is spot on. I consider these strings as something like SVG fragments: you can do text operations on them but you have to be confident that you know what you are doing.
And how could programming in ascii have solved this bug? The problem is that the strings were compared without giving any thought to what "are these strings equal" is supposed to mean.
The only general solution to this kind of bug - that is, to considering distinct emails to be identical - is to have a special function that can check if two emails are identical according to the spec. This function will be different than determining if two names are the same in an American database where dotless and dotful i are the same, and that function will also be different than a function determining if two names are the same in a Turkish database where dotless and dotful i are different. Although I don't know why you're trying to compare strings for identity, you're probably just after similarity anyway - how often do we convert strings to lowercase before we compare them.
Generic string equality functions are always the source of bugs, since there's no useful, single uniformally applicable definition of string equality.
Except for the ones that it really can't. When you try and romanize standard written Chinese it becomes nigh impossible to read for most speakers, often it would be more confusing than an English translation.
>there are quite easy ways to avoid homograph attacks
I'd like to hear about those because as far as I can tell it's still very much an unsolved problem.
It also doesn't help ɡ vs g.
It sounds like any genuine solution is either not simple or excludes legitimate use cases.
Additionally there are 5 additional restriction levels for identifiers depending on your specific situation:
1. ASCII only
2. single script
3. single script or Latin+{Japn, Hanb, Kore}
4. single script or Latin+{any, excluding Cyrillic, Greek}
5. Not containing any characters in the recommended blacklist of characters for use in secure contexts
6. No restrictions other than normal identifier restrictions
For any specific strings that might be confused, there is an algorithm to compute the visual 'skeleton' of a string and match it against that of another string to test if they are confusable.
No. There are these kind of attacks every now and then to this day. Maybe of you're not following itsec they fly under your radar, but getting this right is exceptionally hard. And besides these kind of attack, every major os had multiple bugs just in the processing of Unicode that could at least be used for DoS attacks.
So saying it's easy to avoid any sort of abuse of Unicode seems quite ridiculous.
Go ahead and support it for messages, display names and whatnot, but for the love of god, limit the login name of users to ASCII. Don't assume that your Python/Go/JavaScript lib for Unicode handles sanitizing and canonicalization properly. It doesn't. And even if it has only a minor bug that doesn't lead to direct issues, the next update of the lib might fix the problem and now you have to deal with the fact that your db might contain data that was processed with the old faulty lib and now gets compared to the properly processed output of the new version. Just don't. Use it as opaque data for displaying, as GP said, but never as an identifier for anything.
Bad beancounters did all that. C-levels in suits who got million-dollar bonuses for killing 346 people to gain marginally increased quarterly results. Don't blame the software.
While I frequently point out how bad we are as an industry at making fault tolerant code, this part is just flat wrong.
The software portion of the 737, while definitely flawed in a catastrophic way, would not have came to be if the aeronautics engineers had done their job and designed a flight worthy plane without software hacks. Not to imply the aeronautics guys are the root cause either though, the 737 Max fiasco is a top to bottom completely failure of Boeing as a whole, virtually every department involved in the Max has a significant reason to share in the blame.
You are correct that most of the world is non-ascii and I am enthusiastic about recognizing that diversity and I am willing to pay certain costs in return for the richness it provides.
However, I will point out that certain systems have been deemed crucial and in need of deliberate (and brutal) dumbing-down. Specifically, I speak of the global Air Traffic Control system that is English only[1].
Tagalog/Flemish/Satsugu is hard. Let's (land airplanes with) English.
I wish more programmers understood the value of having a lingua franca is.
UTF-8 should be used everywhere on the user-facing side, but under the hoods, you want unambiguous representation of strings and code. It is easy to avoid homograph attacks if you do have a non-unicode representation available to you.
1) Naive CSV libraries that break when given a field containing a comma itself. Same for injections.
2) Indexing things by strings (file names, user names, etc) relies on humans typing and reading with 100% accuracy.
3) Control characters are still present. Try to generate a file name containing every ASCII character (from 0x01 to 0x7F), then try to delete it in different ways. It's really frustrating.
4) Even string templates can cause problems. MMO's used to have game masters identified with "[GM] Character name", until scammers started using the same pattern. The sin here was to concatenate "[GM] " with the character name, which is spoofable, instead of using a badge or different color.
On Linux, in addition to control characters, the shell interprets the `-` character specially. This means that if a filename starts with `-` you may have surprising results even if the filename is quoted.
It's not the shell that interprets the leading `-` character specially, but rather the program receiving the string. The convention of marking the end of option processing with `--` helps, for programs that support it; you can also prefix relative paths with `./`.
More than half the world’s population does not use the Latin alphabet.
Take a Japanese name like 麻生太郎. He may find his name romanized (ignoring the issue of surname/given name order) as Tarō Asō (preferable), Taro Aso (usually), Taroh Asoh (occurs on passports), Tarou Asou (‘I can't figure out diacritics’-style), Taroo Asoo, or even Tarô Asô.
Case in point: we have punycode for over a decade now. China has their own native TLD. Still pretty much every Chinese website out there gets created under the cn TLD or even com, and uses Latin characters only. How come?
This article doesn't appear well informed, or tries to make the Chinese look stupid. On a similar note there is the common belief that Chinese people, having simplified characters for well over 50 years now, couldn't read the traditional characters still in use in Taiwan and Hong Kong. They are still used for artistic reasons and in calligraphy, which a lot of Chinese people have practiced at some point in their lives. Based on my very limited sample size, I'd argue that a Taiwanese person has more issues reading the simplified characters than vice versa, but then again it's not like all the characters look totally different, so given at least minimal context it shouldn't be too hard going either direction. (end OT rant)
By the way, the first one is not a mail address as @ and @ are different characters. But actually it may be because @ is the @ in the Japanese character set. So it probably should be accepted right? How about if I use the Japanese @ in a korean address? Is that valid? Or a likely attack?
Also in the Arabic name, the first character of the string is the one closer to the @ (it reads right to left), be sure to take that into account in a search interface.
I think it is totally utopian to think that a programmer can know all the subtleties of all the writing systems in the world. I agree that this is a problem that needs to be solved, but users and UX designers should in no way underestimate the magnitudes of additional work it causes.
That is why I like the Rust approach: make these issues front and center, implement clear solutions for common use cases and enforce them. In to many languages encodings feel like an afterthought rather than something that has been considered from the start. When I started using Rust I e.g. learned that the Strings a OS uses in its filenames are not necessarily valid UTF-8. Rust forces you to handle this explicitly. Strings are complex and hiding this can be dangerous
While UTF8 is just an encoding, a Rust UTF8 "String" is actually a UTF8-encoded Unicode 11.0 string. Or version 12.0, possibly 12.1. Maybe 13 soon! Who knows. A moving target, certainly.
If you're "agile", you can just recompile and you will be fine, the next Rust version will surely support any changes in the Unicode standard.
But once a Rust program or crate is no longer actively maintained, many small assumptions will be baked into its Unicode handling at a much lower level than, say, the typical Windows C++ program that uses the OS-provided dynamic libraries for string processing.
Who knows, maybe the consortium will never introduce breaking changes, but I suspect that in a decade or two people will be cursing Rusts too-strong integration with the Unicode of the 2010s...
More to the point: the standard library of Rust relies on no properties of Unicode that will change--there's no builtin normalization, case folding, grapheme cluster support, etc. So there's no data tables in the standard library that need to be updated. The only assumption Rust makes of Unicode is that no assigned codepoints will be changed, and that the space will not grow beyond the current limit of U+10FFFF.
There’s a whole slide deck (filled with real code snippets) with examples where a string was used instead of a better suited object representation and lead to a security flaw.
We eventually build xhp/jsx to get rid of strings-holding-html data, but that was just scratching the surface of bugs caused by user-supplied strings.
Speaking of "evil" broken abstractions...
But when you display a word that contains eight Latin characters and one Cyrillic character, couldn't the display create some sort of warning? Highlight the section-mismatched character with a box? It's not normal in any language to mix alphabets.
Decidedly not true. Languages like French and Turkish and German use their own alphabet but they look like the typical “American” Latin alphabet but aren’t. How do you decide if it’s English with non-American letters thrown in or not? Languages like Arabic are transliterated with English letters and Arabic numerals. Other Transliteration dialects use random Arabic characters with Latin others.
It's hardly bulletproof, but it can certainly stop some of the more obvious shenanigans.
This is not true.
For example, my native language has suffix-based articles (there is no separate word "the"; instead there is a suffix attached to the noun). When including a foreign word in the middle of a sentence, some people will attach the article suffix directly to the foreign word, or at best with a hyphen (this is likely not correct "by the book", but some people write like this anyway). With your suggestion, this will show a warning when it is perfectly normal usage.
Furthermore, some Asian languages like Japanese do not use spaces, so when they mix in foreign words or names written in their original script how do you tell that it's intended to be a separate word?
I don't have a blog yet, but I already have a draft for a post titled "Strings are evil". There's a lot wrong with the current way we develop software, but I'm convinced this is a big one.
The fact that most user input is supplied as a string is a mere coincidence.
Github's mistake in the parent article wasn't overlooking a character casing collision -- it was sending password reset emails to the email provided by the resetter rather than the saved email for that user.
- Only meant to be used in a machine-to-machine interface (like a JSON key for instance).
- Only meant to be used in a machine-to-human interface (like the text letting you know that you used a wrong password).
For strings that have a special significance to both humans and machines you can still run into problems, mainly because of the many unicode strings that look similar (or sometimes even identical) visually but are encoded with a different byte sequence. Take usernames or URLs for instance.
But in the end Unicode is messy because human languages are messy. As programmers we like well-bounded problems with elegant generic solutions. This clearly isn't possible here and we have to deal with it.
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.com. Since you do not lowercase the email and do not recognize him as henry@gmail.com which he used in his last order.
Which would be correct behaviour. From RFC 5321, part 2.4, "The local-part of a mailbox MUST BE treated as case sensitive."
You may not like it but this is fully standards-compliant behaviour. RFC 5321 states:
>The local-part of a mailbox MUST BE treated as case sensitive
I'm not saying this is a good behaviour and the RFC also discourages exploiting it further.
There are more ways an email address can be equivalent though :-). Since we know it's gmail in this case we know the email address is both case insensitive and dots don't matter. Comments are also allowed in email addresses (in both the local part and the domain part). Here's a couple examples from RFC 2822:
pete(his account)@silly.test(his host)
c@(Chris's host.)public.example
[0] https://tools.ietf.org/html/rfc5321For 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.
> 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 from the forgot-password request.
2. Get the email address from the database for the same account.
3. Check whether they match.
4a. If not, we're under attack -- refuse the request.
4b. If so, all is well -- send a password reset to the email address.
Of course, we know the email address twice -- we asked the user for it during the password reset process (step 1), but we never needed to do that because we already had an email address on file for the account. We retrieved that email address in step 2. We know that the two addresses are the same, but, if you look at the semantics behind the variables, in step 4b we're choosing one of these two "equivalent" options, depending on which variable we use for the email address:
1. Send the account password to the account owner.
2. Send the account password to a guy who doesn't know what the password is.
And these have very different risk profiles. Choosing the first option instead of the second would have prevented this attack without needing to worry about unicode case-translation issues. You never want to trust information you just received from an unknown user when you already have the same information from a more authoritative source.
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 user.email_address
You can see the problem in the call to reset_password. Sure, it's the same as user.email_address, but if you just used user.email_address you'd be doing the right thing and you wouldn't need the comment about "don't worry; it's fine".Then you start to wonder if that line shouldn't look more like
reset_password(user)
and from a security perspective, it should. (I assume the reason Github allows the code to specify the email address in the first place is that an account may be associated with multiple addresses.)If it makes no difference which email address you pick, why not pick the one that represents doing something safe instead of the one that represents doing something dangerous?
``` 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.
Tripple backticks do nothing, and will be shown on the same line as the next line even if put on a separate line, because they are considered as regular text and part of one paragraph of text.
If the code were instead written:
username = request.post_params('username')
email = request.post_params('email_address')
user = get_user_by_name(username)
if emails_are_equal(email, user.email_address)
reset_password(user.id, email)
Most people would not feel compelled to use user.email_address in the call to reset_password, because they're guaranteed to be equal by the previous line! The vulnerability was introduced when the author of emails_are_equal() wrote it with conversion to uppercase of the whole thing, and failed to realize that this conversion can collide with Unicode domains. They may also have done things like normalize/remove optional.dots+suffixes@gmail.com, "quoted" or (commented) local parts. There's a lot of magic - and a lot of risk - in an emails_are_equal method.My whole point is that changing reset_password(user.id, email) to reset_password(user.id, user.email_address) neutralizes that risk. Then it doesn't matter whether emails_are_equal is risky or not, because a failure in emails_are_equal can only cause you to do a safe thing. What's less embarrassing -- "we accidentally sent a password reset for your account to you", or "we accidentally sent a password reset for your account to someone else"?
So you might not feel compelled to rewrite reset_password(user.id, email) as reset_password(user.id, user.email_address) -- but you should.
At least the first two (removing dots and suffixes), along with conversion to lowercase, are strictly invalid transformations which may result in an email address referring to a different account. Sure, most email providers treat the account name as case-insensitive, and the use of '+' as a label/subaccount separator is a common convention, but neither of these is required. The RFCs say that the account name is case-sensitive (unlike the domain name) and '+' is just an ordinary part of the name; any special significance is assigned by the server. Ignoring '.' characters is something specific to Gmail.
In the context of a search it's not unreasonable to ignore some of these differences, but at that point the matching names aren't "equal", just "similar". Certainly it should never be assumed that it's safe to send email to the transformed version of the address.
I believe I understand the rest (the take-away being, however you match A to B, send the reset email to the email address stored in the DB?), just not sure about the flow beforehand.
In step zero, you enter "2T1Qka0rEiPr" as the username of the account you want to hack.
In step one, github says "We have m------@e------.com on file for you. Please confirm your email address." and you enter "myemaıl@example.com".
Then github retrieves the email address associated with the username "2T1Qka0rEiPr".
You're correct that you could do this with just an email address and not a username, but that doesn't affect my criticism -- you'd still want to ultimately send the reset to the email address you pulled from the database, not the one you got from the reset request. Your takeaway is exactly right.
Another more secure method is to pull up the account information and display "We have the following methods on file for contacting you: () email 1 (potentially obscured); () email 2 (potentially obscured); () SMS (to a phone number which is potentially obscured)". If I recall correctly, that's how Twitter does it. This bypasses the need to ask the user to type in the address they'd prefer for the reset to be sent to -- you just show them some radio options, and they select the one they want. Since they never provided the address, there's no chance you'll accidentally pick their malicious address over the real address.
"But how do I make sure someone knows the email address, in order to stop strangers from spamming reset emails to addresses they might not even know?" You don't; you apply rate limiting to the reset functionality.
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)
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)
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.)
You are right that TypeScript aliases don't work like Haskell ones (which are considered different, and type checked). In TypeScript you can use "branded types" to work around the more loose structural typing:
type Firstname = string & { readonly brand?: unique symbol }
https://stackoverflow.com/a/51578232nope. they are not byte arrays. if treating them as such, you run into many nasty bugs. today I encountered one (C#):
if (string.Length > 60)
string.Remove(60)
this has a fucking serious bug, especially if you try to show the string afterwards or worse save it inside a database.strings are arrays of codepoints. NEVER TREAT THEM AS BYTE/CHAR ARRAYS.
> because the system lowercased the provided email address and compared it to the email address stored in the user database.
While sending the email to the attack-provided email, instead of the one in the database, is bad… lowercasing emails is also not valid. The lookup should never have matched in the first place.
(It's slightly more complicated: to an extent, the case of the domain name doesn't matter, ignoring non-ASCII characters — I have no idea what they do. But the local part — the portion before the @ — is case sensitive. A server is free to ignore that, and map multiple local parts to the same mailbox internally¹, and many do, but the sender cannot make that assumption.)
¹or do other weird things, like ignore dots, or +extensions, etc.
I usually give my email address as starting with a capital letter for historical reasons, and I once had an issue that wouldn't let me log on with any case whatsoever, probably because it lowercased the email address and tried to match it against the uppercase in the database.
You still need to normalize the unicode for either search or display, unless you're only allowing ascii or something.
E.g. for the column you store for search and to guarantee uniqueness, casefold and then normalize to NFKC. And for the column you store for display, normalize to NFC. (And obviously you need to sanitize user input before doing anything.)
The local part MUST be treated as case-sensitive, servers are discouraged from doing so (as Gmail does): https://tools.ietf.org/html/rfc5321#section-2.4
Github shouldn't have normized case, but it's not insane to require lowercase in the first place I think so long as you don't convert silently.
I wouldn't call +extensions and ignoring dots weird because Gmail does that and fairly or unfairly they set the standard for user expectations of email nowadays.
Also, I think it's unreasonable to require full compliance with the spec. For example, if you go around trying to give out email addresses with comments in the ("usern(ignored_comment)ame@example.com") you'll see many things break and I don't think that's an issue.
For a delightful example of how insane email addresses can get, here's a fully compliant regex to validate one:
(?:(?:\r\n)?[ \t])(?:(?:(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t] )+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))"(?:(?: \r\n)?[ \t]))(?:\.(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:( ?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))"(?:(?:\r\n)?[ \t])))@(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\0 31]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\ ](?:(?:\r\n)?[ \t]))(?:\.(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+ (?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?: (?:\r\n)?[ \t])))|(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z |(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))"(?:(?:\r\n) ?[ \t]))\<(?:(?:\r\n)?[ \t])(?:@(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\ r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t]))(?:\.(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n) ?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t] )))(?:,@(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t])* )(?:\.(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t] )+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t])))) :(?:(?:\r\n)?[ \t]))?(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+ |\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))"(?:(?:\r \n)?[ \t]))(?:\.(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?: \r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t ]))"(?:(?:\r\n)?[ \t])))@(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031 ]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\]( ?:(?:\r\n)?[ \t]))(?:\.(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(? :(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(? :\r\n)?[ \t])))\>(?:(?:\r\n)?[ \t]))|(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(? :(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)? [ \t]))"(?:(?:\r\n)?[ \t])):(?:(?:\r\n)?[ \t])(?:(?:(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]| \\.|(?:(?:\r\n)?[ \t]))"(?:(?:\r\n)?[ \t]))(?:\.(?:(?:\r\n)?[ \t])(?:[^()<> @,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|" (?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))"(?:(?:\r\n)?[ \t])))@(?:(?:\r\n)?[ \t] )(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\ ".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t]))(?:\.(?:(?:\r\n)?[ \t])(? :[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[ \]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t])))|(?:[^()<>@,;:\\".\[\] \000- \031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|( ?:(?:\r\n)?[ \t]))"(?:(?:\r\n)?[ \t]))\<(?:(?:\r\n)?[ \t])(?:@(?:[^()<>@,; :\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([ ^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t]))(?:\.(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\" .\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\ ]\r\\]|\\.)\](?:(?:\r\n)?[ \t])))(?:,@(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\ [\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\ r\\]|\\.)\](?:(?:\r\n)?[ \t]))(?:\.(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\] |\\.)\](?:(?:\r\n)?[ \t])))):(?:(?:\r\n)?[ \t]))?(?:[^()<>@,;:\\".\[\] \0 00-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\ .|(?:(?:\r\n)?[ \t]))"(?:(?:\r\n)?[ \t]))(?:\.(?:(?:\r\n)?[ \t])(?:[^()<>@, ;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(? :[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))"(?:(?:\r\n)?[ \t])))@(?:(?:\r\n)?[ \t])* (?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\". \[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t]))(?:\.(?:(?:\r\n)?[ \t])(?:[ ^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\] ]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t])))\>(?:(?:\r\n)?[ \t]))(?:,\s( ?:(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\ ".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))"(?:(?:\r\n)?[ \t]))(?:\.(?:( ?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[ \["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))"(?:(?:\r\n)?[ \t ])))@(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t ])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t]))(? :\.(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+| \Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t])))|(?: [^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\ ]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))"(?:(?:\r\n)?[ \t]))\<(?:(?:\r\n) ?[ \t])(?:@(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\[" ()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t]))(?:\.(?:(?:\r\n) ?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<> @,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t])))(?:,@(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@, ;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t]))(?:\.(?:(?:\r\n)?[ \t] )(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\ ".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t])))):(?:(?:\r\n)?[ \t]))? (?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\". \[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))"(?:(?:\r\n)?[ \t]))(?:\.(?:(?: \r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\[ "()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))"(?:(?:\r\n)?[ \t]) ))@(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t]) +|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t]))(?:\ .(?:(?:\r\n)?[ \t])(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z |(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)\](?:(?:\r\n)?[ \t])))\>(?:( ?:\r\n)?[ \t]))))?;\s)
http://www.ex-parrot.com/~pdw/Mail-RFC822-Address.html
Edit: Apparently Perl is wrong. Thanks https://news.ycombinator.com/item?id=21810662 !
You might not be surprised that regex comes from a Perl module :)
Edit: The following is completely wrong
For example, the python module to parse an email address is around 500 lines and repeatedly warns it'll be very hard to follow without a copy of the spec in front of you. It contains code for parsing multiple timezone formats and cite to a follow up spec addressing a bug in the initial treatment of negative timezones...
https://github.com/python/cpython/blob/master/Lib/email/_par...
The code I wrote for parsing email headers is here: https://github.com/jcranmer/jsmime/blob/emailutils/headerpar... . A decent chunk of it is building a full lexer for email headers, and trying to cope with only supporting internationalization support in a few cases where they need to be supported. And the corner cases for that i18n support are really nasty.
An email address consists of a local-part, a literal @ character, and then a domain name or an IP address literal. The local-part is either a series of dot-separated atoms (/[a-zA-Z0-9!#$%&'+/=?^_`{|}~-]+/ is the syntax for an atom) or a quoted string (/"([^\\"\0-\031\x7f]|\\[^\0-\031\x7f])"/ in regex). If you support EAI, you need to add \u00a0-\u10ffff [i.e., all non-ASCII non-C1 control characters].
According to RFC 822, you can insert whitespace (and comments) arbitrarily into the mailbox production, but that is non-semantic. Seeing "From: John Doe <foo @ example (I ((hate)) CFWS).com>" means that the email address is exactly "foo@example.com", and you can reject any claim of the spelling without prejudicing any email addresses.
In practice, you can drop all support for quoted strings and IP address literals in most applications. So a correct email address (pre-EAI) regex in that vein would be:
[a-zA-Z0-9!#$%&'*+/=?^_`{|}~-]+(.[a-zA-Z0-9!#$%&'*+/=?^_`{|}~-]+)*@([a-zA-Z0-9]+(-[a-zA-Z0-9]+)*\.)+[a-zA-Z]{2,}
If you insist on quoted-string support, then it looks like this: ([a-zA-Z0-9!#$%&'*+/=?^_`{|}~-]+(.[a-zA-Z0-9!#$%&'*+/=?^_`{|}~-]+)*|"([^\\"\0-\031\x7f]|\\[\\"])*")@([a-zA-Z0-9]+(-[a-zA-Z0-9]+)*\.)+[a-zA-Z]{2,}
[I simplified the quoted string to only accept escapes that are semantically necessary--the strings "a b"@example.com and "a\ b"@example.com correspond to the same email address].Edit: Sorry, there's quite a few asterisks in the regexes that Hacker News is turning into italicization, and I don't know how to unbork them.
Edit 2: Someone suggested how to unbork the standalone regexes, but the asterisks in the inline regex in the second paragraph are still missing.
Prefix the regex with four spaces.
user = get_user_from_valid_email(params[:email])
send_reset_email(params[:email])
# instead of
# send_reset_email(user.email)
?I've seen this pattern before and the reason is usually something about using the variable in memory as opposed to the function call. Total non-optimisation.
An apparently small deviation with surprisingly large repercussions.
We accomplish this with a slightly more restrictive version of the standard ABNF provided in the RFC.
I guess I should probably document why we go to this trouble, in case somebody gets the brilliant idea to "simplify" it.
The period thing is a Gmail feature, not a standard. some.email@mydomain and someemail@mydomain most certainly do not deliver to the same mailbox.
That's why we keep your verified mailbox address for sending mail; but there's no good reason to consider them different for the purpose of identity.
A false postive in these identity checks is likely to be less destructive than a false negative. But I still don't get the point of making up all sorts of rules not in the standard. I have seen both + as well as meaningful dots in e-mail adresses in the wild.
Google was the first installation I know of to silently swallow periods. Plus-addressing was well-known back in the day, but as far as I know GUI mailers more or less killed the practice by not offering support for it, and web sites written by people too smart to know how to validate email addresses ensured you can't even use them properly anymore.
Ref: http://www.faqs.org/rfcs/rfc822.html, pages 8/9.
Both of Microsoft's own identity services (AAD and Live ID, and by extension O365 and Outlook.com) recognize (and allow creation of) microcolonel@example.com, micro.colonel@example.com, and mic.rocolonel@example.com as distinct identities/email addresses.
What you're saying is that once the first of (microcolonel|micro.colonel|mic.rocolonel)@example.com registers at github, the other two will no longer be able to do so, but will instead receive a confusing 'you already have an account' error, (hopefully) without being able to receive password reset emails.
For example, a malicious user can register
some.email@gmail.com
so.meemail@gmail.com
someem.ail@gmail.com
so.mee.mail@gmail.com
somee.mail@gmail.com
so..mee.mail@gmail.com
som..eem.ail@gmail.com
so.meemail@gmail.com
somee.mail@gmail.com
with a service, which will (quite reasonably) send a "Welcome" email. That results in a flood of emails to the GMail user.And the only way it can be automated is if the service doesn't protect itself against automated user sign-ups, which they will either start doing once someone really takes advantage of it, or will result in their domain being categorised as spam once they start sending lots of sign-up emails (either by the user or gmail in general).
What is a problem, and what is non-standards-compliant, is GitHub incorrectly assuming that all mail providers will do this when many do not. It would be no different from assuming that "admin" and "postmaster" go the same mailbox because that's the way a lot of software is configured.
I doubt we'll ever turn away a customer by preventing registration of a new account sharing the prefix to a plus sign in their email address with an existing customer.
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 repeat, this has absolutely nothing to do with the mailbox address, where we send mail.
It might make them bad netizens and you may not like it. But the spec doesn't compel any behavior. It's just a way to communicate technical ideas and ideals.
You're under no obligation to accept names that are hard (as in painful) for you (except if you're the police, I suppose. Ironically)
You can't break user expectations and mental models by pointing to the spec as justification. The spec exists to serve users, not the other way around.
In the absence of being able to count on specs, I guess the user should expect a race?
Most of them can't specifically point to the problem, their impression is just that your services don't work properly. They're right.
Assuming they don't just fail to register and move on ...
It's like deciding to not allow quoting and with this whitespace. Sure ":"@example.com is a legal mail address (surprised?) but nothing good will come from allowing it.
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?
So at best you can special case for those "major hosts" and not apply such treatment to any other domain.
The RFCs for email don't say "uhh dude whatever, just check what gmail does and maybe hotmail too lol". You are playing fast and loose with these things.
So far it was only a gut feeling. Now I know why I do it ;)
Lots of non-thought too. Sending e-mail directly from the place where web requests are processed isn't very smart. What if the SMTP subsystem is currently down or very slow? How many e-mails will you send if an attacker starts 1000 parallel web requests for a password reset?
A saner way to do these things is to just set a flag ("password reset requested") in the user database there and do the actual work asynchronously in a regularly performed maintenance task. It'll also prevent all these attacks based on misuse of user input by default (unless, in this case, you pointlessly decide to update the user's e-mail in the user database to what the hacker specified).
I expect things on the web to be damn near instant - I want to click that password reset button and hear a synchronous "ding" of an arriving mail. I don't want to wait for some cron job to run once per minute.
If you want to send mail off the serving path, at least use a push based queue so there is no scheduling or polling involved.
e.g. GitHub gets this request, queues the job in redis/zero MQ/SQS or whatever they're using, and another process dedicated to sending those emails (or jobs with that priority) does the rest of the work.
This is a massively common pattern in the Rails world and is as trivial to configure as your database connection.
The first case (yours) is a plain bug, while the latter (mine) is an architecture bug. Architecture bugs often arise out of organizational bugs.
Of course, email providers are free to do whatever case folding or normalization they want, in which case the security burden of avoiding collisions is on the provider. If someone's email provider maps different case variants to the same mailbox, there's still no need whatsoever to do anything to the address, as the user will get it delivered to them regardless. If the provider doesn't do case folding, they will have to enter their local part case sensitively, but that's exactly the same as for any other use of their email address.
I can only imagine how this vulnerability came to be. Unicode is not to blame here. If security-critical password reset code was not audited carefully enough to catch a mistake like this, one wonders what other errors might remain.
Why? First of all ß is already lowercase, why should toLowerCase() change it? It also is a normal letter having both uppercase (ẞ) and lowercase (ß) forms so converting between the cases can be made be trivial and quirk-free. Arguably the most common word you will encounter ß in is Straße (street) where it already is lowercase - will "Straße".toLowerCase() turn it into "strasse"? WTF? "Straße".lower() returns "straße" in Python which seems reasonable (nevertheless "Straße".upper() actually returns "STRASSE" ignoring the existence of the uppercase ẞ (U+1E9E)). Why should it behave different in JavaScript? (Because JavaScript is different, I know, just a rhetorical question)
Python probably predates the addition of a notional capital ß glyph in 2017. SS is the capitalization of ß that you'd expect if you were thinking of your data as a string rather than a collection of font elements.
More likely is a bunch of young and/or not too bright people were looking for a reason to get into a violent confrontation. Then the muckraking Turkish press had a sensationalist murder-suicide lovers quarrel story, and as a bonus a nationalistic "see how cell phone companies don't respect our culture" angle as the cherry on top.
However, you should still ALWAYS be careful when converting between character sets and be locality aware when manipulating strings. Practice string safety. ;)
I'm not sure this example is correct. The dotless ı is already lower cased, so the comparison above should yield false. Maybe the author was thinking about upper case dotted "İ", which becomes regular dotted "i" when lower cased.
So what could happen is that an user enter "JOHN@GİTHUB.COM" as email, and then the email is sent to "john@github.com" .
The best a researcher can do is go back 20+ years and look for security issue that occurred back then. Most likely you'll find very similar things now again.
It's `'ß'.toUpperCase()` that is `"SS"`, not `'ß'.toLowerCase() === 'ss'`. As the later chart makes clear. Same with turkish ı.
'ß'.toUpperCase() // = "SS"
'ß'.toLowerCase() // = "ß" "ẞ".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)[0] https://www.unicode.org/reports/tr21/tr21-3.html#Caseless%20... [1] http://userguide.icu-project.org/transforms/casemappings#TOC...
https://labs.spotify.com/2013/06/18/creative-usernames/
I had someone tell me that programming isn't real work before, and this is yet another example of all the small little details going into building things that most people don't really think about day to day.
I haven't had to work with login code in a while, but might at some point. I know some systems only allow alphanumeric usernames, but sounds like can't force people to have alphanumeric emails... Well I guess you could but might upset someone. I know there's normalizing functions though like NFD, NFC, NFKD or NFKC that might work for usernames but not sure what's really recommended.
Then also brute forcing attempts to try to mitigate attacks and other considerations to make also when building out an account system. Then if your company is large enough to provide phone support, not sure how you'd tell the support person which specific emoji you used in your username.
Haha, I don't even understand what metrics the person was using to consider something "work". A pilot sits throughout the flight but I think it's fair to say their working...
Now that I think about it as far as I remember the local part of mail is actually not defined as cases insensitive , through all? mail programs treat it as such. The important part her is to always use data from your database for any security relevant parts.
I fail to see any added value in sending a reset link to an e-mail entered by the user (while that e-mail is already in the database).
Is it because the e-mails are stored hashed ?
>'ß'.toLowerCase() // 'ss'
"ß"
>'ß'.toLowerCase() === 'SS'.toLowerCase() // true
false
>// Note the Turkish dotless i
>'John@Gıthub.com'.toUpperCase() ===
'John@Github.com'.toUpperCase()
true
Chrome 79.0.3945.79 (Windows, 64bit) seems to differ from the proposed results for the first two statements. If the proposed results are what should indeed happen by unicode standard then I wonder chrome is not fully implementing them? julia> c='ı'
'ı': Unicode U+0131 (category Ll: Letter, lowercase)
julia> uppercase(c)
'I': ASCII/Unicode U+0049 (category Lu: Letter, uppercase)
julia> lowercase(uppercase(c))
'i': ASCII/Unicode U+0069 (category Ll: Letter, lowercase)
Is this something that needs changing in the Unicode spec itself or how strings are handled in general by various tools/programming languages? I love [plain] text, but it's so, so fragile. :/Why would you write a general function that resets an account password but also accept an email address as a parameter? What use-case exists to change the email address sending the message?
[0] https://www.djangoproject.com/weblog/2019/dec/18/security-re...
http://whale.hacking-lab.com:8881/
I'm not sure if the solution is unicode collisions like in the article - still trying to solve it...
Fraktur had/has other such lower case ligatures (tz, ch, sch, ss (not ß) et al) but for some reason only ß survived into Latin script as a full fledged letter. I have a lot of old (mostly 20th century) books in Fraktur and they all use these ligatures more consistently than the Latin ligatures are used.
Some sort of orthographic device is needed - s between two vowels means the first vowel is long and the consonant is voiced, and ss between two vowels means the first vowel is short and the consonant is voiceless. So it's useful to have a different case for when the first vowel is long and the consonant is voiceless:
Busen /bu:zən/
Busse /busə/
Buße /buːsə/
Vowel length isn't predictable from spelling in cases of consonant clusters and other digraphs: Hand /hant/ vs. Mond /moːnt/, Bruch /brux/ vs. Buch /buːx/, etc. So ss could've been used for both - or sz, although there are some words spelled with sz as a sequence of s and z, like Szene /stseːnə/.
this is sarcasm right?
I don't believe Microsoft can permit things like this under it's umbrella if it wants to continue to pretend that it's data-collection is benign.
If you are at interested in Turkish characters:
https://en.wikipedia.org/wiki/Wikipedia:Turkish_characters
https://www.turkcebilgi.com/türkçe_karakter
This should be called the Turkish character hack: A hack only possible in the theoretical realm.
These characters are rendered in HTML and supported by OSes. They are just not used for emails AND website domains.
So the password reset “hack” with an email containing Turkish characters in not a possibility from the get go.
The whole attack vector hinges on emails that exist with Turkish characters in the first place.
Turkish emails are not supported in the first place.
Internationalization examples[edit] The example addresses below would not be handled by RFC 5322 based servers, but are permitted by RFC 6530. Servers compliant with this will be able to handle these:
Latin alphabet with diacritics: Pelé@example.com
Greek alphabet: δοκιμή@παράδειγμα.δοκιμή
Traditional Chinese characters: 我買@屋企.香港
Japanese characters: 二ノ宮@黒川.日本
Cyrillic characters: медведь@с-балалайкой.рф
Devanagari characters: संपर्क@डाटामेल.भारत
It is true that the RFC recommends mailbox providers take normalization into account. A mailbox provider that allows i and dotless-i addresses to be routed to different mailboxes is careless, if not actually uncompliant. I don't know if any popular provider does this: I'm guessing the authors created their own to demonstrate this attack.
There are no email addresses with Turkish characters at all. They all use Latin characters.
It just does not exist - yet at least.