13 ms·

Mongo BSON Injection: Ruby Regexps Strike Again

128 points by homakov 11y ago

jpatokal 11y ago
Egor Homakov strikes again. This guy is a machine, just look at the vulnerabilities he's found this year alone: http://sakurity.com/blog http://sakurity.com/blog
- atonse 11y ago
  Oh boy I'd love to watch this guy in action in a twitch.tv stream, taking down a site (to clarify... white hat stuff!). It would be so damn fascinating to see how his mind works and how he makes the leap from 0 to exploit.
  - icpmacdo 11y ago
    Is there anything currently like this for other people?
    - tobeportable 11y ago
      I've been thinking about live coding on twitch but having to think about not revealing any sensitive information would make it a hassle. I will give it a try for some side projects.
  - mikekchar 11y ago
    For this kind of stuff, I'm not sure it would be that interesting. Lots of staring off into space... scrolling the source code... cursing people who can't write code that other people can understand... oh! That doesn't look right... Only the "oh!" part would be about 20 hours into the stream ;-) I once had an idea to invite open source developers to remotely pair with me for a day on something (a bug fix, some feature they are working on, whatever)... I would record the session, do an introduction to the problem, and then edit the session down to about an hour or an hour and a half. I think it would be fascinating (if a lot of work). Maybe some day...
  - pjc50 11y ago
    That sounds unfeasibly dull, compared to the nice text explanation. You can see what someone's typing but that really isn't what's in their head.
- sarciszewski 11y ago
  Yeah, he's impressive. Hats off to his persistence and ingenuity. Would love to collaborate with him sometime.
  - elcct 11y ago
    No, let's keep white hat on...
    - sarciszewski 11y ago
      What exactly are you accusing me of?
      - robflynn 11y ago
        I think you mis-parsed the response. You said "hats off" and elcct was making a joke based on that re: white hats. I don't think there was any accusation intended.
        sarciszewski 11y ago
        Oh, I thought he was calling me whitehat scum. Thanks for clarifying.
- mambodog 11y ago
  Not to mention this classic, which was probably his original claim to fame: https://news.ycombinator.com/item?id=3663197 https://news.ycombinator.com/item?id=3663197
jasonmp85 11y ago
> 3 years ago I wrote a blog post about broken regular expressions in Ruby, ^$ meaning new lines \n. Are they actually broken? One of the first quirks I learned writing Ruby is that you use \A and \z instead of ^ and $. I know better than to blame security vulnerabilities on "bad programmers" rather than usability problems with a language (or plain old PoLA violations), but changing the meaning of these anchors will be a tough migration, possibly.
- zer01 11y ago
  "Broken" is a subjective thing, but quirks that break extremely well established patterns (like how people validate the beginning and end of a line in regex) is where a lot of security issues tend to crop up. Considering that I would venture to say that this is indeed a broken thing since it's a convention that I've only heard about in Ruby. It's a special snowflake in a component that developers use almost exclusively for validation (regex), with a pretty huge gotcha, especially because it appears to function as intended.
  - cremno 11y ago
    Ruby inherited this behavior from Emacs (matz' favorite text editor) among other things.
- pyre 11y ago
  While I tend to agree with you, how many people have tried to downplay C's deficiencies with "good programmers following good practices don't introduce buffer overflows into their code?" Where has that gotten us? I'm not saying that I have a solution, but I can't help be see the parallels.
- developer1 11y ago
  Why is everybody talking about ruby's regex implementation? ^, \A, $, \Z, \z are standard across many languages, based on PCRE syntax. This isn't a ruby problem; this is simply developers who are not experts with regular expressions not fully understanding how to write patterns with the tool they are using. There are no problems with regex implementations, only problems with regex use. Anybody interested should have a read through http://pcre.org/pcre.txt http://pcre.org/pcre.txt. The syntax presented here is used in perl, php, ruby, python, and many others. Also, nobody ever uses \A without the /m flag. You use ^, it has the same meaning unless you specifically add the /m flag to allow ^ to match at the beginning of any line rather than at the front of the string only. This distinction will only bite developers who just add flags like /msig for every regex, because again they don't understand exactly what every flag actually does.
cheald 11y ago
The 1.x versions of BSON are vulnerable, too, FWIW.
- rsutphin 11y ago
  I tested on our app (which uses BSON-ruby 1.9.2) and was surprised to find that the detection code indicated it was not vulnerable. Turned out it was because we also use bson_ext — bson_ext replaces the vulnerable method with a C implementation which doesn't use regexes.
  - cheald 11y ago
    Oh, that's a good catch. I checked on JRuby, which doesn't use bson_ext.
  - ploxiln 11y ago
    Kinda funny to see a "safe" language saved by C. Just sayin'
gabeio 11y ago
To clarify this isn't Mongo's BSON, it's Moped's implementation of BSON/Ruby's implementation of BSON (again). The title is fairly misleading making it sound like it's actually Mongo which is vulnerable. Still interesting stuff though.
- rsutphin 11y ago
  The article describes how this problem was present in Moped's BSON implementation, then fixed. Then later, Moped replaced its own BSON implementation with BSON-ruby, which had a version of the method which was not vulnerable. Later still, BSON-ruby's method was changed, making it vulnerable. BSON-ruby is, AFAICT, the official BSON library for ruby from Mongo. As I understand it, the vulnerability is in any ruby application which uses a vulnerable version of the bson gem and which accepts object IDs from user input. You don't have to be using Moped.
  - deleted 11y ago
    [deleted]
- mbell 11y ago
  This is not correct. The vulnerability is in `bson-ruby`[1] which is written by MongoDB and used by Moped (and thus Mongoid), the official Ruby driver from MongoDB, and Mongo Mapper. The only thing that _isn't_ vulnerable is Moped's BSON implementation (if reasonably recent), but it was dropped in Moped 2.x. In reality if your using Mongo with Ruby, your most likely vulnerable, unless you happen to be on Moped 1.x. [1] https://github.com/mongodb/bson-ruby/blob/84d8acd32ce9067ad646755e52f472a6ad685918/lib/bson/object_id.rb#L285 https://github.com/mongodb/bson-ruby/blob/84d8acd32ce9067ad6...
  - gabeio 11y ago
    > This is not correct. > The vulnerability is in `bson-ruby`[1] which is written by MongoDB and used by Moped (and thus Mongoid), the official Ruby driver from MongoDB, and Mongo Mapper. Then it's in the ruby gem of MongoDB's driver for ruby NOT in MongoDB. The title is still misleading for people who do not code in ruby and therefore are not vulnerable to the apparently ever present ruby BSON bug. > Mongo BSON Injection A better title would be Mongo gem BSON Injection I am not trying to nit-pic I was fairly confused when seeing the title because I don't code in ruby and was 99% sure Mongo's core was C not ruby.
jph 11y ago
One way to help protect this kind of issue is to be explicit about validation steps. This is the buggy code: !!str.match(/^[0-9a-f]{24}$/i) That regex is trying to do three different things: validate the length is 24, validate the string contains alphanums, and ensure the matching is pinned from start to finish. I prefer code that makes the validation steps explicit and simpler: str.length==24 && str!~/[^0-9a-z]/i
- deleted 11y ago
  [deleted]
  - simcop2387 11y ago
    the [^ part negates the characters inside the []. So it'll match anything that is NOT 0-9a-z, case insensitive /i. The !~ then says that str should not match the regex. so you end up with it saying that str should not match anything other than 0-9a-z case insensitive.
    - deleted 11y ago
      [deleted]
- arielby 11y ago
  Its a whitelist. It is "verify the string is a 24-character alphanumeric string".
deleted 11y ago
[deleted]
hnanon6 11y ago
I suspect that many of these same Rubyists who mistakenly assumed ^ and $ have the normal PCRE semantics would still readily claim to know Ruby, and in many cases, even to know it well. I think this type of vulnerability undermines the optimistic belief that any decent programmer can quickly learn a new language, and further, it shows the danger of adopting new languages generally, especially those with extremely complex and not particularly well-defined syntax and semantics, like Ruby. I also wonder how many vulnerabilities result just from Rubyists favoring cutesy APIs (or "DSLs," as they call them) that while making for great demos, hide the often times unignorable, crucial details of what they do from their users.
- rubygemcrap 11y ago
  Yep, ruby tard brogrammers strike again. Our company criteria for any new tool is simply: is it written in ruby? If yes, then run away from it (crap such as rails, homebrew, etc.) because it will be awful insecure code by people who have no business even pretending to be developers. Even code monkeys flinging digital feces would produce better code.
maerF0x0 11y ago
Here's one lurking in javascript: /A-z/ includes "[]^_`" [1] Now go search github [2] and see the +1k repos that have this bug in their parsing of base64 Sources: [1]: http://wtfjs.com/2014/01/29/regular-expression-and-slash http://wtfjs.com/2014/01/29/regular-expression-and-slash [2]: https://github.com/search?utf8=%E2%9C%93&q=INVALID_BASE64_RE&type=Code&ref=searchresults https://github.com/search?utf8=%E2%9C%93&q=INVALID_BASE64_RE...
- pxndx 11y ago
  This isn't WTF at all, that's exactly how regexes should work!
  - kiallmacinnes 11y ago
    In case anyone is wondering why this isn't a WTF - See [1].. [1]: http://www.asciitable.com/ http://www.asciitable.com/
  - maerF0x0 11y ago
    agreed, not my website so i couldnt control the name. Just talking about how A-z includes non(standard) base64 chars
- sandstrom 11y ago
  Interesting! I didn't understand it at first, but the key difference is `A-z` vs. `A-Za-z`.
gshutler 11y ago
Where's the attempt to submit a patch to fix the problem before disclosing?
allcentury 11y ago
Is this only an issue if you are defining the BSON _id ?
eddanger 11y ago
Added a patch for Moped::BSON in Rails here https://gist.github.com/eddanger/9408317d5d508d8e9ba7 https://gist.github.com/eddanger/9408317d5d508d8e9ba7
veesahni 11y ago
This has been fixed. Just update moped: gem "moped", "~> 2.0.5"

128 points by homakov 11y ago

Egor Homakov strikes again. This guy is a machine, just look at the vulnerabilities he's found this year alone: http://sakurity.com/blog http://sakurity.com/blog

atonse 11y ago
Oh boy I'd love to watch this guy in action in a twitch.tv stream, taking down a site (to clarify... white hat stuff!). It would be so damn fascinating to see how his mind works and how he makes the leap from 0 to exploit.
- icpmacdo 11y ago
  Is there anything currently like this for other people?
  - tobeportable 11y ago
    I've been thinking about live coding on twitch but having to think about not revealing any sensitive information would make it a hassle. I will give it a try for some side projects.
- mikekchar 11y ago
  For this kind of stuff, I'm not sure it would be that interesting. Lots of staring off into space... scrolling the source code... cursing people who can't write code that other people can understand... oh! That doesn't look right... Only the "oh!" part would be about 20 hours into the stream ;-) I once had an idea to invite open source developers to remotely pair with me for a day on something (a bug fix, some feature they are working on, whatever)... I would record the session, do an introduction to the problem, and then edit the session down to about an hour or an hour and a half. I think it would be fascinating (if a lot of work). Maybe some day...
- pjc50 11y ago
  That sounds unfeasibly dull, compared to the nice text explanation. You can see what someone's typing but that really isn't what's in their head.
sarciszewski 11y ago
Yeah, he's impressive. Hats off to his persistence and ingenuity. Would love to collaborate with him sometime.
- elcct 11y ago
  No, let's keep white hat on...
  - sarciszewski 11y ago
    What exactly are you accusing me of?
    - robflynn 11y ago
      I think you mis-parsed the response. You said "hats off" and elcct was making a joke based on that re: white hats. I don't think there was any accusation intended.
      - sarciszewski 11y ago
        Oh, I thought he was calling me whitehat scum. Thanks for clarifying.
mambodog 11y ago
Not to mention this classic, which was probably his original claim to fame: https://news.ycombinator.com/item?id=3663197 https://news.ycombinator.com/item?id=3663197

jasonmp85 11y ago

> 3 years ago I wrote a blog post about broken regular expressions in Ruby, ^$ meaning new lines \n. Are they actually broken? One of the first quirks I learned writing Ruby is that you use \A and \z instead of ^ and $. I know better than to blame security vulnerabilities on "bad programmers" rather than usability problems with a language (or plain old PoLA violations), but changing the meaning of these anchors will be a tough migration, possibly.

zer01 11y ago
"Broken" is a subjective thing, but quirks that break extremely well established patterns (like how people validate the beginning and end of a line in regex) is where a lot of security issues tend to crop up. Considering that I would venture to say that this is indeed a broken thing since it's a convention that I've only heard about in Ruby. It's a special snowflake in a component that developers use almost exclusively for validation (regex), with a pretty huge gotcha, especially because it appears to function as intended.
- cremno 11y ago
  Ruby inherited this behavior from Emacs (matz' favorite text editor) among other things.
pyre 11y ago
While I tend to agree with you, how many people have tried to downplay C's deficiencies with "good programmers following good practices don't introduce buffer overflows into their code?" Where has that gotten us? I'm not saying that I have a solution, but I can't help be see the parallels.
developer1 11y ago
Why is everybody talking about ruby's regex implementation? ^, \A, $, \Z, \z are standard across many languages, based on PCRE syntax. This isn't a ruby problem; this is simply developers who are not experts with regular expressions not fully understanding how to write patterns with the tool they are using. There are no problems with regex implementations, only problems with regex use. Anybody interested should have a read through http://pcre.org/pcre.txt http://pcre.org/pcre.txt. The syntax presented here is used in perl, php, ruby, python, and many others. Also, nobody ever uses \A without the /m flag. You use ^, it has the same meaning unless you specifically add the /m flag to allow ^ to match at the beginning of any line rather than at the front of the string only. This distinction will only bite developers who just add flags like /msig for every regex, because again they don't understand exactly what every flag actually does.

cheald 11y ago

The 1.x versions of BSON are vulnerable, too, FWIW.

rsutphin 11y ago
I tested on our app (which uses BSON-ruby 1.9.2) and was surprised to find that the detection code indicated it was not vulnerable. Turned out it was because we also use bson_ext — bson_ext replaces the vulnerable method with a C implementation which doesn't use regexes.
- cheald 11y ago
  Oh, that's a good catch. I checked on JRuby, which doesn't use bson_ext.
- ploxiln 11y ago
  Kinda funny to see a "safe" language saved by C. Just sayin'

gabeio 11y ago

To clarify this isn't Mongo's BSON, it's Moped's implementation of BSON/Ruby's implementation of BSON (again). The title is fairly misleading making it sound like it's actually Mongo which is vulnerable. Still interesting stuff though.

rsutphin 11y ago
The article describes how this problem was present in Moped's BSON implementation, then fixed. Then later, Moped replaced its own BSON implementation with BSON-ruby, which had a version of the method which was not vulnerable. Later still, BSON-ruby's method was changed, making it vulnerable. BSON-ruby is, AFAICT, the official BSON library for ruby from Mongo. As I understand it, the vulnerability is in any ruby application which uses a vulnerable version of the bson gem and which accepts object IDs from user input. You don't have to be using Moped.
- deleted 11y ago
  [deleted]
mbell 11y ago
This is not correct. The vulnerability is in `bson-ruby`[1] which is written by MongoDB and used by Moped (and thus Mongoid), the official Ruby driver from MongoDB, and Mongo Mapper. The only thing that _isn't_ vulnerable is Moped's BSON implementation (if reasonably recent), but it was dropped in Moped 2.x. In reality if your using Mongo with Ruby, your most likely vulnerable, unless you happen to be on Moped 1.x. [1] https://github.com/mongodb/bson-ruby/blob/84d8acd32ce9067ad646755e52f472a6ad685918/lib/bson/object_id.rb#L285 https://github.com/mongodb/bson-ruby/blob/84d8acd32ce9067ad6...
- gabeio 11y ago
  > This is not correct. > The vulnerability is in `bson-ruby`[1] which is written by MongoDB and used by Moped (and thus Mongoid), the official Ruby driver from MongoDB, and Mongo Mapper. Then it's in the ruby gem of MongoDB's driver for ruby NOT in MongoDB. The title is still misleading for people who do not code in ruby and therefore are not vulnerable to the apparently ever present ruby BSON bug. > Mongo BSON Injection A better title would be Mongo gem BSON Injection I am not trying to nit-pic I was fairly confused when seeing the title because I don't code in ruby and was 99% sure Mongo's core was C not ruby.

jph 11y ago

One way to help protect this kind of issue is to be explicit about validation steps. This is the buggy code: !!str.match(/^[0-9a-f]{24}$/i) That regex is trying to do three different things: validate the length is 24, validate the string contains alphanums, and ensure the matching is pinned from start to finish. I prefer code that makes the validation steps explicit and simpler: str.length==24 && str!~/[^0-9a-z]/i

deleted 11y ago
[deleted]
- simcop2387 11y ago
  the [^ part negates the characters inside the []. So it'll match anything that is NOT 0-9a-z, case insensitive /i. The !~ then says that str should not match the regex. so you end up with it saying that str should not match anything other than 0-9a-z case insensitive.
  - deleted 11y ago
    [deleted]
arielby 11y ago
Its a whitelist. It is "verify the string is a 24-character alphanumeric string".

deleted 11y ago

[deleted]

hnanon6 11y ago

I suspect that many of these same Rubyists who mistakenly assumed ^ and $ have the normal PCRE semantics would still readily claim to know Ruby, and in many cases, even to know it well. I think this type of vulnerability undermines the optimistic belief that any decent programmer can quickly learn a new language, and further, it shows the danger of adopting new languages generally, especially those with extremely complex and not particularly well-defined syntax and semantics, like Ruby. I also wonder how many vulnerabilities result just from Rubyists favoring cutesy APIs (or "DSLs," as they call them) that while making for great demos, hide the often times unignorable, crucial details of what they do from their users.

rubygemcrap 11y ago
Yep, ruby tard brogrammers strike again. Our company criteria for any new tool is simply: is it written in ruby? If yes, then run away from it (crap such as rails, homebrew, etc.) because it will be awful insecure code by people who have no business even pretending to be developers. Even code monkeys flinging digital feces would produce better code.

maerF0x0 11y ago

Here's one lurking in javascript: /A-z/ includes "[]^_`" [1] Now go search github [2] and see the +1k repos that have this bug in their parsing of base64 Sources: [1]: http://wtfjs.com/2014/01/29/regular-expression-and-slash http://wtfjs.com/2014/01/29/regular-expression-and-slash [2]: https://github.com/search?utf8=%E2%9C%93&q=INVALID_BASE64_RE&type=Code&ref=searchresults https://github.com/search?utf8=%E2%9C%93&q=INVALID_BASE64_RE...

pxndx 11y ago
This isn't WTF at all, that's exactly how regexes should work!
- kiallmacinnes 11y ago
  In case anyone is wondering why this isn't a WTF - See [1].. [1]: http://www.asciitable.com/ http://www.asciitable.com/
- maerF0x0 11y ago
  agreed, not my website so i couldnt control the name. Just talking about how A-z includes non(standard) base64 chars
sandstrom 11y ago
Interesting! I didn't understand it at first, but the key difference is `A-z` vs. `A-Za-z`.

gshutler 11y ago

Where's the attempt to submit a patch to fix the problem before disclosing?

allcentury 11y ago

Is this only an issue if you are defining the BSON _id ?

eddanger 11y ago

Added a patch for Moped::BSON in Rails here https://gist.github.com/eddanger/9408317d5d508d8e9ba7 https://gist.github.com/eddanger/9408317d5d508d8e9ba7

veesahni 11y ago

This has been fixed. Just update moped: gem "moped", "~> 2.0.5"