[HN Gopher] Sign in as anyone: Bypassing SAML SSO authentication...
       ___________________________________________________________________
        
       Sign in as anyone: Bypassing SAML SSO authentication with parser
       differentials
        
       Author : campuscodi
       Score  : 138 points
       Date   : 2025-03-15 19:06 UTC (3 hours ago)
        
 (HTM) web link (github.blog)
 (TXT) w3m dump (github.blog)
        
       | oncallthrow wrote:
       | XML is to authentication bypasses what C is to buffer overflow
       | attacks
        
         | dietr1ch wrote:
         | Sad that XML has too many features for an otherwise somewhat
         | nice, but verbose markup language.
        
           | treve wrote:
           | Feature are kind of a negative for security. Imagine if yaml
           | was used!
        
             | alexchamberlain wrote:
             | I think there is a "safe" subset of both XML and YAML that
             | 80% of people actually use.
        
               | bawolff wrote:
               | From a security perspective that's kind of useless, as
               | your concern is not what the "good" people do, it's what
               | the "bad" people do.
        
               | alexchamberlain wrote:
               | Well, you can define such a subset and write or configure
               | parsers to only use that; I've seen both XML and YAML
               | libraries do just that, by disabling remote file loading
               | or arbitrary code execution for example.
        
               | bawolff wrote:
               | Disabling xml remote entities and billion laughs is a
               | given.
               | 
               | In the context of saml that's hardly the least of it.
               | Lots of the problems are things like allowing comments to
               | sort of change the meaning of the document, allowing
               | signatures to sign only part of the document. Allowing
               | multiple signatures to sign different parts of the
               | document, etc.
        
               | Muromec wrote:
               | which is exactly the problem. if you have two parsers of
               | the same format in a security context that show slightly
               | different behavior (maybe in the rest 20% or maybe not)
               | it's often enough.
        
           | bawolff wrote:
           | Some of it isn't explicitly XML's fault (although it doesn't
           | help). SAML and especially XMLSignature are terrible
           | standards even in ways that dont involve xml.
        
         | pvg wrote:
         | You're selling XML short here, it had its own share of straight
         | up RCEs too.
        
       | derektank wrote:
       | >We discovered an exploitable instance of this vulnerability in
       | GitLab, and have notified their security team
       | 
       | GitLab has released a fix on their end for anyone else wondering
       | 
       | https://about.gitlab.com/releases/2025/03/12/patch-release-g...
        
       | bawolff wrote:
       | Its kind of annoying to explain the vulnerability in a blog post
       | and then omit the parser differential in question.
       | 
       | It is like writing the introduction to a story and omitting the
       | climax.
        
         | mdaniel wrote:
         | The sibling comment's blog post
         | <https://news.ycombinator.com/item?id=43374972> included the
         | relevant detail: they were just doing
         | (...//ds:DigestValue).firstChild.nodeValue without checking
         | that .firstChild was a Node (in the offending case, it was a
         | Comment). Thus, the non-canonical one saw the "masked"
         | signature, the corrected one which tossed out comments saw a
         | Node and when two implementations differ about a signed
         | document hilarity will ensue
        
           | bawolff wrote:
           | Are you sure that is the one for this blog post? i got the
           | impression that was a different vuln for a different saml
           | implementation.
           | 
           | Also using comments to bypass saml is very old news.
           | https://duo.com/blog/duo-finds-saml-vulnerabilities-
           | affectin... is a post from 2018 about it.
        
             | mdaniel wrote:
             | Evidently it's not the same, sorry; it seems that I lept to
             | conclusions with the two signature mismatch vulns by
             | ahacker1 showing up so close to one another but opening the
             | very tiny, very dark, code picture shows this seems to be
             | xpath-centric, not nodeType as the workos link discussed
        
       | noleary wrote:
       | This is a great write-up.
       | 
       | He's mentioned in the article, but a major shout-out is warranted
       | for ahacker1. He's doing really sophisticated and valuable work
       | to secure SAML implementations. We at SSOReady are really
       | appreciative of his work.
       | 
       | Earlier this week, WorkOS put together a nice write-up on their
       | own collaboration with ahacker1:
       | https://workos.com/blog/samlstorm
        
       | Diggsey wrote:
       | I recently had to implement SAML and this headline does not
       | surprise me in the slightest.
       | 
       | The SAML spec itself is fairly reasonable, but is built upon XML
       | signatures (and in turn, XML canonicalization) which are truly
       | insane standards, if they can even be called such.
       | 
       | Only a committee could produce such a twisted and depraved
       | specification, no single mind would be capable of holding and
       | combining such contradictory ideas.
       | 
       | It would be _so_ simple to just transmit signatures out-of-band
       | and SAML would be a pleasure to implement.
        
         | jiggawatts wrote:
         | It's much worse than you're making it sound: XML is literally
         | an eXtensible Markup Language, so... of course the SAML
         | standardisation committee invented their own extension
         | mechanism language on top of it.
         | 
         | Coming up with your own protocol on top of a protocol for a
         | tiny amount of data amounting to not much more than what's in
         | an authentication cookie is the special kind of stupid that
         | only the largest and most bureaucratic committees can produce.
        
       | wcoenen wrote:
       | Isn't the simpler conclusion here that one should look for the
       | signature where it is supposed to be? Instead of using an
       | excessively general XPath like "//ds:Signature" that might find
       | any signature in any unexpected location...
        
         | Muromec wrote:
         | Hot take, but for me the conclusion always was -- get a big
         | stick and use it to prevent web developers from touching
         | anything near your security sensitive code. Starting from
         | design, protocols and data formats of it. The set of habits and
         | design considerations simply doesn't match day to day practice
         | of the usual web development. It's often the opposite of what
         | you need to write normal code.
        
       | asmor wrote:
       | GitHub's SAML implementation is useless. The idea is that you can
       | bring your own account into an enterprise, and that sort of works
       | on the site itself, but it does not prevent apps where you log in
       | with GitHub from reading your organization membership once you
       | have authorized an app _on the organization level_ (and if you
       | didn 't, it hides the membership from oauth tokens, so it has
       | this capability!).
       | 
       | A SAML session is only required if said app fetches data via a
       | token obtained from that user - and in my glance around, this was
       | almost never the case - SAST tools almost always use app instance
       | tokens and are happy to show anyone with a GitHub account in your
       | organization your code. Tailscale fixed this when I pointed it
       | out, Sonarcloud told me to please don't tell anyone and GitHub
       | took a few weeks to say this is totally expected behavior - when
       | no vendor I told did, and their docs contradicted them.
       | 
       | I swear, reporting security bugs is a thankless endeavor, even if
       | you just randomly stumble over them. I couldn't imagine doing
       | this as a job.
        
         | weard_beard wrote:
         | This is the operating procedure at every conceivable level. You
         | would not believe how difficult it is to convince young
         | developers raised on Javascript that client side validation is
         | not enough, much less the business owners setting out
         | functional requirements and budgets.
        
           | dboreham wrote:
           | Sorry to go all Musk and all, but those developers should be
           | fired.
        
             | weard_beard wrote:
             | They'd be fired if they spent a penny more fixing it.
        
             | UltraSane wrote:
             | Musk should be fired. He has irreversibly destroyed the
             | Tesla brand.
        
               | woleium wrote:
               | But has he realized yet?
        
         | eCa wrote:
         | > The idea is that you can bring your own account into an
         | enterprise
         | 
         | The issues goes beyond authorization. I've had Github randomly
         | once in a blue use my personal email address as the default
         | when merging a work PR. If anyone asks, I advice against mixing
         | personal and professional stuff in the same Github account (or
         | anywhere).
        
       | kayodelycaon wrote:
       | Ugh. No one should use REXML unless they have no other choice. It
       | will happily parse invalid xml, which causes an infinite number
       | of problems downstream.
       | 
       | It's quite literally parsing xml using regular expressions. It's
       | an excellent case study for why you shouldn't do that.
       | 
       | Projects didn't start using Nokogiri for performance. They used
       | it because it's correct.
        
         | mtkd wrote:
         | One of risks of AI code assistance is that they are not
         | necessarily looking at the wider picture when it comes to
         | libraries used on a large code base
         | 
         | I was testing o3 recently and it kept changing the library used
         | by a block of code every time it tried to fix an issue in the
         | block that was unrelated to the library used (haven't seen that
         | happen with Sonnet)
         | 
         | Easy to see how issues could creep in because a modification is
         | made that switches to an inferior library/gem that exists in
         | the code base or standard library so still passes tests etc.
         | but doesn't need a Gemfile change
        
       | kinow wrote:
       | BlueSky post with a video showing the vulnerability:https://bsky.
       | app/profile/ulldma.bsky.social/post/3lkbi6rasl2...
        
       | tptacek wrote:
       | SAML (more broadly XML-DSIG) is literally the worst security
       | protocol in common use. I think you should generally be taking
       | whatever hits you need to take to transition from it to OAuth.
       | Certainly, I would refuse to bring a new product to market that
       | relied on it. It's incredibly dangerous. Unless there's some
       | breakthrough in practical formal verification, I can't imagine
       | that this will be the last or the worst DSIG vulnerability.
        
         | shellcromancer wrote:
         | Security Cryptography Whatever's take on this week SAML non-
         | sense will be fun.
        
           | tptacek wrote:
           | Honestly, hadn't thought of it, but of course we should do
           | that. Thanks!
        
       | GauntletWizard wrote:
       | Saml is insecure by design. Others have said it better before me,
       | such as https://joonas.fi/2021/08/saml-is-insecure-by-design/,
       | but the quote I got from an old thread here was "Sign Bytes, not
       | meanings".
       | 
       | Parser differentials are expected and even necessary. What you
       | intend to get from a signed response is very meaningful. A
       | dilemma in modern TLS is that sometimes you want to trust one
       | internal CA; That's the easy path. Sometimes you want to accept a
       | certificate from a partner's CA, and you've got multiple partners
       | - and you can no longer examine just the end certificate, but the
       | root of that chain is equally important in your decisions.
       | 
       | This is also why I recommend whenever possible against AWS Sig
       | algorithms; V4 is theoretically secure, but they screwed it up
       | twice - SigV1 and SigV3 were insecure by design, and yet somehow
       | made it past design review and into the public.
        
       | malkia wrote:
       | Maybe asking stupid question, but would older versions of puppet
       | be affected (like 6?). Also is there a site to check deps down to
       | what maybe affected?
        
       ___________________________________________________________________
       (page generated 2025-03-15 23:00 UTC)