When str.lower() is a security vulnerability in Python

(sethmlarson.dev)

174 points | by rbanffy a day ago ago

83 comments

  • echoangle a day ago

    > This is why calling str.lower() represents a difference in the implementation and the specification, and therefore a vulnerability:

    I wish there was some explanation how this is a vulnerability and not just a bug generating erroneous data.

    Vulnerability for me sounds like there’s a reasonable way to create an exploit from the bug, and I don’t see one here as someone who’s not very familiar with the topic.

    • tptacek a day ago

      It creates a parser differential; two different components of the system can treat the same string as different hostnames. Things that have trusted hostnames, or privileged/admin hostnames that are screened out, or SSRF filters all depend on accurately comparing presented hostnames.

      This is pretty situational, though, isn't it? You still have to be dealing with IDN names.

      • tialaramex a day ago

        It is situational, but it very much seems like a thing you'd squirrel away and bring out when you find a system where the differential is helpful.

        DNS names are a thing where Sales is going to tell the Engineer that they can't issue the customers randomized ASCII names like abxuewrf.my-thing.example because real customers want to write our-brand-name.my-thing.example instead - even though you already know bad guys will choose billing.my-thing.example and name-of-bank.my-thing.example and every other unintended bad choice even before we realise about likelihood of these confusion bugs in software like Python.

        • xp84 a day ago

          That's why for that type of 'semi-white-label' thing, since the main risk is one of impersonating the platform owner (like the billing.my-thing.example) or possibly lending the credibility of "our" brand to some rando UGC, I always push for the most boring and generic second-level domain, like if it's the travel business, 'travel-systems dot us' or in edtech, mylearningplatform dot net... Then push all customers who know what they're doing into 'BYO DNS name' anyway.

          I also like how sites like github use githubusercontent.com or something like that when linking to UGC assets directly, to avoid someone direct linking to something with the implication that it's coming from GitHub.

          • chuckadams a day ago

            It's mostly about not sharing a security context with github.com.

            • cwillu a day ago

              Yes, but github.cc or whatever would have sufficed for that; there's a reason the string they chose to register for prominent public use contains “usercontent”

              • inigyou a day ago

                literally github.io though

                • notpushkin 19 hours ago

                  And before that one, user content was hosted on *.github.com! Wild times.

          • inigyou a day ago

            That's actually to prevent cookie stealing.

            • lozenge 19 hours ago

              Yes, involving the Public Suffix List. It's quite ridiculous you need to register your domain on a list of you want to offer subdomains.

        • inigyou a day ago

          Solution: customer emails you to request a name

          • zrm 20 hours ago

            How does the customer service rep tell that a name with some Unicode gubbins is an attack rather than a customer from Juárez or 서울? Having a busy hand copy and paste the attacker-provided string into the system doesn't get you out of it.

            • wildzzz 14 hours ago

              Simple but less culturally sensitive solution: English is the linga franca so tell them to either romanize their company name or come up with a brand name using latin characters.

              삼성전자 can be written as Samseong jeonja in latin characters and translated literally as Tristar Electronics but they do business as samsung.com

              I think it's cool that we figured out a way to encode CJK and other foreign script into ASCII-only domain names but if there's a serious concern about opening yourself to domain spoofing because someone picked the wrong encoding, just use ASCII characters.

            • inigyou 15 hours ago

              Do you sell to Korea? If so, ask the Korea team. If not, don't allow it.

            • schluss-s 17 hours ago

              "Juárez" can be done with ASCII tho

              • tialaramex 16 hours ago

                > "Juárez" can be done with ASCII tho

                No it can't? Are you thinking of ISO-8859-1 or Windows codepage 1252 maybe?

          • 21 hours ago
            [deleted]
      • belorn 14 hours ago

        Not all Unicode characters are permitted in DNS, so the parser differential need to exist within the allowed set of Unicode characters. For example, a parser differential on a "space" unicode character won't impact a list of trusted hostnames, since space are not allowed in the first place.

        Looking at examples where it could matter, German Sharp S (ß) is allowed in domain names and so a list of trusted hostnames could be fooled if one hostname had a double ss idn-encoded name and the attacker registered a second domain with ß, but there is a caveat here. Looking at how .eu registry handle it, they will not allow two domains to only have the difference between ss and ß, unless they are the same registrant and explicitly request to have both variants (https://eurid.eu/en/knowledge-centre/domain-names-with-speci...). Thus the attack vector will depend on if the registry has protection against it or not.

      • pseudohadamard 2 hours ago

        It's a big deal in anything dealing with permitted/excluded names because once you take the insane C/Posix locale rules into account you end up with nondeterministic comparison results: compare a with b, get a match, another thread changes the locale setting, compare the same a with b, get a non-match. You've also got nontransitive case-changes, e.g. toupper('ı') = 'I', tolower('I') = 'i', 'ı' != 'i'. This means that an attacker can get around any name-based checks by choosing a character set that compares however they want it to, which is why a lot of OSS security projects implement their own guaranteed-deterministic comparison functions, e.g. WireGuard's built-in replacement for the ctype.h macros/functions.

      • cobbzilla 17 hours ago

        IDN is a security flaw masquerading as a standard.

      • raverbashing 21 hours ago

        From what I understand the vulnerability is not on lower() but on using different Unicode rules version for it

        That sounds to me like a better explanation

      • TZubiri a day ago

        I can see how this would be a vulnerability in the context of a security researcher that wants to exaggerate their findings in order to get paid a bounty.

    • SethMLarson a day ago

      Author here, that's a good idea. A straightforward way to exploit an implementation differential like this is if you have a software system that contains two different implementations of IDNA 2003 processing user input. One part of the process processes the domain correctly, the other incorrectly, and in this case you can have one part of a system (such as a policy/filter) "see" the data one way and the other part of the system (such as, taking an action as a result of the data) see the data in another way.

      Server-side Request Forgery (SSRF) is an example of such an exploit targeting a differential in implementations of URL parsers, which is similar to this implementation difference.

      • jerf a day ago

        I wouldn't call this a "vulnerability", I'd call it "a thing that can potentially turn into a vulnerability, more often it can turn into an obscure bug, and most often it is just a quirk".

        In particular, if my corporate security team started just mass-flagging all instances of "str.lower" as "security bugs" I would be having a talk with their manager about their threshold for what constitutes a "security bug". Their job is arguably to be more sensitive to that than most engineers, but not that sensitive. It would be like flagging all instances of string concatenation as a vulnerability... and I say that as the guy who would like to eliminate simple string concatenation from programming languages, already a very extreme position on that operation, because of it being at the root cause of so many vulnerabilities... but simply flagging every use as a "vulnerability" is way too sensitive. A demonstration of the ability to use it to bypass some sort of security barrier is necessary to call any specific instance a "vulnerability".

        And string concatenation has caused orders of magnitude more actual, verified vulnerabilities than incorrect case folding has.

        • catlifeonmars a day ago

          What’s a way to flag to an engineering team that they should do a thorough review of their usage of a particular API because it has footguns in it?

          This is a rhetorical question because there isn’t a generally accepted way of doing so. Automatically patch everything is a silly way to do vulnerability management but software is cheap to change, so it’s often easier at scale to just force engineering teams to patch even if it doesn’t make sense in context.

          I’m not a fan of this approach, but I can understand why it’s so popular.

          • masklinn a day ago

            You could have a CO which emits overridable warnings or requires additional / specific reviewers.

            Of course that can then lead to warnings fatigue so it’s not necessarily a big improvement, or an improvement at all, in the long run, depends a lot on the org philosophy and habits.

            • catlifeonmars 3 hours ago

              Warning fatigue is a very good callout. I find it’s really easy to get to a state where you are inundated by notifications that aren’t actionable.

      • reichstein a day ago

        I guess that's why you shouldn't be _validating_ things, and then praying them on as-is. "Parsing is validation" is (also) the idea that after validation, your data is in a format that reflects and enforces what's known about it.

        "Safe strings" is an example of that idea. Not always possible or practical, but always worth considering if you're doing "validation" as a function.

      • gcr a day ago

        That would be a vulnerability in the IDNA filter that they’re responsible for fixing.

      • AgentOrange1234 a day ago

        "if you have a software system that contains two different implementations of IDNA 2003 processing user input"

        Is that a real thing though? Is someone doing that?

        • kccqzy a day ago

          It could be an implementation written in the buggy Python and another written in a different language.

          For example you might use a ready-made WAF written in a non-Python language in front of a Python app.

        • rcxdude a day ago

          With web applications it's not particularly unusual, because the whole system stack can be quite heterogeneous. If one part of the system is doing authentication and the other part is actually doing the action then it can be a real problem when they interpret the input differently. Differences between proxy and web server interpretations of HTTP headers have been a source of multiple vulnerabilities, for example.

          • dmurray 21 hours ago

            It's not particularly unusual in Python, because it's normal that important functionality is implemented in other languages by a diverse set of third parties.

            That said, this isn't a security vulnerability, it's just a bug. To meet a reasonable threshold for being a security issue, you need to show a real system that has an issue caused by this, and then the vulnerability is in that system, rather than in Python.

            I'll grudgingly allow that a buffer overflow or an SQL injection possibility - in a library advertised as safe against that kind of bug - is a security issue, because there's so much history of turning those into real exploits. But a choice of library or language that makes those bugs easier to write - the idna library, or C or PHP say, is not itself a security issue.

        • cwillu a day ago

          Consider the case where your system has components in python and another language without the bug, both of which process that input.

        • gchamonlive a day ago

          It isn't until it is, until during a crunch someone adds a package with that condition and eventually that gets exploited or halts the system. It's never a nitpick to shed your system from undesired state because of how complex systems behave.

    • wren6991 a day ago

      My favourite example of this is the Chromium bug where enabling floating point flush-to-zero for WebAudio was used to cause deliberate heap corruption: https://issues.chromium.org/issues/382005099

      > We have a working exploit (OOB access in the V8 heap), our security folks put one together based on the example I posted above (and they're cleaning it up to post it here). In general, we find that correctness issues like this are pretty much always exploitable with a bit of effort (not even that much effort normally, just gluing together a few gadgets), so we treat correctness issues as security issues until they are proven not to be, rather than the other way around.

      The floating-point-to-heap-corruption chain here is... uniquely JavaScript, but in general getting two different implementations to disagree is the start of lots of interesting inconsistent behaviour.

    • floxy a day ago

      I suppose I could envision a scenario like: Service A has a "reset your password" option. Someone with a "user@popular-unicode-domain.com" tries to use this option. Normally this fails when mangled-popular-unicode-domain-plus-garbage.com can't be found. Enterprising malicious actor registers mangled-popular-unicode-domain-plus-garbage.com, and now gets a hold of user password reset requests.

    • zaphar a day ago

      I'm too lazy to investigate further but my guess is that if there is a vulnerability here it has something to do with dns name spoofing.

    • amelius 18 hours ago

      Fingerprinting comes to mind.

    • raverbashing a day ago

      Honestly it seems it's grabbing at straws

      There are a whole bunch of more consequential vulnerabilities before worrying about that

  • tialaramex a day ago

    This idiocy is a big part of why it was so important to get Python people working on TLS implementations to understand that the defined mechanism for SANs (no the "alternative" in Subject Alternative Name doesn't mean in the sense of more than one, X.509 is originally for the X.500 system and the Internet repurposed X.509 so these are alternative names from the Internet) says that these are DNS names, they specifically are not to be understood as some sort of human readable text, and thus "decoding" them to Unicode is definitely nonsense even though Python really wanted to do that and I think used to do it or at least proposed to.

    The rule for how SAN DnsNames match againt like names, from the DNS is very, very simple so that you don't screw it up. You handle a single wildcard (ASCII * code 42 matches any single DNS label) and beyond that it's literally byte comparison. You don't care what these bytes mean, either the bytes are all identical or that's not a match and we're done.

    • ltpoll 17 hours ago

      This sums up the whole Unicode disaster in Python3. People are conditioned to "encode" and "decode".

    • MattSteelblade 16 hours ago

      But it's not literally byte comparison; it's case-insensitive ASCII comparison.

  • ummonk a day ago

    > The fix was to create new exceptions so that str.lower() would behave as if it was using Unicode 3.2.0 for only particular function. So, we go through each Unicode codepoint and record when the behavior of str.lower() is different when comparing the Unicode version shipped with Python and Unicode 3.2.0

    This sounds like a really hacky solution compared to implementing a separate frozen Unicode 3.2.0 lower.

    • jwilk a day ago

      The first sentence sounds as if they modified the implementemention of str.lower(). That would be bonkers, but that's not what they did.

      https://github.com/python/cpython/commit/7e109d084d55e7eb

      The important part is:

         # B.3 is mostly Python's .lower, except for a number
         # of special cases, e.g. considering canonical forms.
        +# To enforce Unicode 3.2.0 behavior of .lower instead of
        +# whatever Unicode version is included with Python we
        +# add unassigned or newly case-folding codepoints to
        +# the exception map, too.
         
         b3_exceptions = {}
         
         for k,v in table_b2.items():
             if list(map(ord, chr(k).lower())) != v:
                 b3_exceptions[k] = "".join(map(chr,v))
        +for cp in range(0x110000):
        +    ch = chr(cp)
        +    # Assigned in current Unicode version
        +    # and supports case folding, but not
        +    # explicitly in B.2 or B.3 tables.
        +    if (unicodedata_current.category(ch) != "Cn"
        +            and ch.lower() != ch
        +            and cp not in table_b2
        +            and cp not in table_b3):
        +        b3_exceptions[cp] = ch  # Identity.
      • quietbritishjim 20 hours ago

        That fragment doesn't mean much in isolation. You've just said that they didn't modify str.lower (because "that would be bonkers") but you've posted a fragment which, for all we know, is part of the str.lower implementation.

    • philipwhiuk 18 hours ago

      To be clear (because the snippet is non-explanatory).

      For encode("idna") what they did is use lower() except where it would produce a result different to 3.2.0 and then instead use the result from 3.2.0 instead.

      Essentially they've frozen the IDNA encoding to be based on 3.2.0 by overriding any changes.

      • ummonk 13 hours ago

        Yeah, that's what I figured, but my worry upon seeing this is "what happens if lower() changes again and people forget to update the list of exceptions?".

        Unless they have unit testing on the entire Unicode code space to ensure what they're doing is always identical to 3.2.0.

      • ryukoposting 17 hours ago

        Yeah. I can understand the confusion, though. The title claims the issue was in lower(). Though the problem was actually in encode('idna')'s usage of lower().

        The article would probably get far fewer clicks if it were named "when encode('idna') is a security vulnerability"

  • ghusbands 18 hours ago

    So someone used lower() from an unspecified version of Unicode when the standard was very specific about which to use. And they say "There's also a database of Unicode 3.2.0 data available on every version of Python (unicodedata.ucd_3_2_0) specifically for the StringPrep and IDNA algorithms", so the right version is available.

    And then the fix is to hardcode a bunch of special cases which again depend on exactly which version of Unicode is in use, and so will break again in the same way in future, rather than just using the right version?

    • akoboldfrying 14 hours ago

      I'd assumed that the updating of the B3 dict takes place at runtime, either during module initialisation or lazily at the first call (that is, roughly as late as possible -- certainly, well after the wheel was built), just as a perf optimisation.

      But then I realised the code does (and must do) a lookup in the B3 table for each character anyway, so there doesn't seem to be any point. I suppose it means they can load the full 3.2.0 table once, use it to discover the exceptions and then immediately evict it from memory, keeping only the presumably smaller and faster-to-query B3 table of exceptions, but this seems pretty marginal...

  • jooon a day ago

    Reminds me of an old security incident at Spotify https://engineering.atspotify.com/2013/06/creative-usernames

  • K0IN a day ago

    I was also startled when python did ß.upper() returns "SS". Which is kind of unsuspected in some cases (if string length changes with an upper call)

    • smallerize a day ago

      That's in the standard. https://www.unicode.org/reports/tr21/tr21-5.html

      [SpecialCasing] Contains additional case mappings that map to more than one character, such as "ß" to "SS".

      • kevin_thibedeau a day ago

        5.1 adds uppercase ẞ which can fold to either ss or lowercase ß depending on the chosen algorithm.

        • reichstein a day ago

          Yes, but for backwards compatibility, Unicode doesn't uppercase ß to ẞ.

    • cpach a day ago

      Seems kinda logical, since that’s how the Germans did it for a long time? ẞ wasn’t introduced until 2017 or so, AFAICT.

  • the_black_hand 18 hours ago

    This could apply to any two systems that don't follow the same standard. not really a security issue, but the moral of the story is makes sure all components of your system use the same standard.

  • hun3 15 hours ago

    Browsers (WHATWG) uses UTS #46, not IDNA2008.

    ada-url (https://github.com/ada-url/ada-python) closely tracks WHATWG spec and thus less likely to lead to parser differential vulnerabilities when interoperating with browsers.

  • ajd555 a day ago

    So the attack surface would be a bit flip on a domain name? Or more specifically, a unicode conversion flip, where an attacker could redirect to a malicious IP?

    Impressive to have found such a vulnerability!

  • rurban 15 hours ago

    Are there any security relevant international domain names, which require punycode? That's the real security problem: dont use IDNA 2003 for anything security relevant. redirect to an ASCII domain.

  • philipwhiuk 18 hours ago

    If this is a vulnerability every bug in every API is a vulnerability.

    This is very spacebar-heating.

    It's not a vulnerability, it's a bug. A system that used this bug in a way that relied on it to perform a security task would have a vulnerability.

    We need to stop seeing library functions that are not themselves security systems as having vulnerabilities.

    • dazed_confused 13 hours ago

      How else are we supposed to have claims that we are finding all the vulnerabilities in the world?

  • bawolff a day ago

    That's a little over the top. string.lower() is not a security vulnerability. Not following the spec is the security vulnerability.

    • orphea 17 hours ago

      This is a typical clickbait title

  • cat-whisperer a day ago

    the horrors of versioning

  • ike_sh a day ago

    Hit this with the Kelvin sign once. Took embarrassingly long to track down.

  • drdexebtjl a day ago

    All 6 users of IDNs must be really worried right now.

  • inigyou a day ago

    If this is so important to know you probably shouldn't serve 403 errors to people.

  • tescreal a day ago

    it would be good to look for ".lower()" squatters if you host on an idn.

  • ittsel a day ago

    if you have a software system that contains two different implementations of IDNA 2003 processing user input" Is that a real thing though? Is someone doing that?

  • ittsel a day ago

    Is that a real thing though?

  • trsthales 19 hours ago

    [flagged]

  • manganate06 16 hours ago

    [flagged]

  • syamjith_nk 17 hours ago

    [flagged]

  • Daniel-Pan a day ago

    [dead]

  • neet_dev 21 hours ago

    [dead]

  • kevinbaiv a day ago

    [dead]

  • myshapeprotocol 14 hours ago

    [dead]

  • marsx-dev 18 hours ago

    [dead]

  • Ozzie-D a day ago

    [flagged]

  • ltpoll 17 hours ago

    Python itself should not be used at all to handle any server, or heaven forbid, TLS connections.

    The Python core developers barely know anything about SSL apart from aggressively updating openssl versions and making a big show of the "security" implied by the updates.

    Naturally the jocks [1] from AlphaOmega sponsor this bottomless pit. They didn't give curl access to Mythos, but if you talk the talk, projects beyond repair get money.

    My advice to college students: Create a completely broken project with a big foundation and many president and director titles. Talk and network at conferences until you get money to perpetually fix the mess.

    [1] A look at their website should disabuse dreamers of the recent notion that nerds and "weird" people will be in charge in the future.