
ActiveRecord Vulnerability - Circumvention of attr_protected - craigkerstiens
https://groups.google.com/forum/?fromgroups=#!topic/rubyonrails-security/AFBKNY7VSH8
======
tptacek
I want to shamelessly give a shout-out to Ryan from our MTV team on this one;
Rails ActiveModel was I think? the first real piece of Ruby code he ever
looked at, and he found the permset Blacklist regex bypass (joernchen found
the other one) inside of an hour. Everyone here will testify that I was no
help to him at all; my contributions mostly consisted of throwing a large
rubber balancing ball at him from the other side of the office.

Finding people like Ryan to work with--- "sleepers" who don't have long track
records of filing public vulnerabilities, but who are secretly terrifying
killing machines --- is the single best thing about my job.

Ryan found us on HN after beating the Stripe CTF. If you're like Ryan, ping
me; I will go out of my way to make sure we tell you everything we can about
why you should work with us.

Back on topic: if I was going to place a bet on something about Rails
security, it'd be that there are more regex vulnerabilities in the tree. I am
uncomfortable with how much Rails leans on regex for policy decisions.

Finally, said this before, saying it again: if you have a Rails app with
customers, you need to be following @joernchen on Twitter full stop.

~~~
bradleyland
I want to send a big thank you to everyone looking closely at Rails right now.
I really appreciate all the work that's being done. I can't tell you how happy
it makes me to know that Rails becomes more secure every day because of
efforts by teams like yours.

~~~
tptacek
I think Phenoelit is doing more than anything else to motivate us right now.

------
calinet6
Shortly followed by two others:

[https://groups.google.com/forum/?fromgroups=#!topic/rubyonra...](https://groups.google.com/forum/?fromgroups=#!topic/rubyonrails-
security/KtmwSbEpzrU) (Rails 2.3 and 3.0)

[https://groups.google.com/forum/?fromgroups=#!topic/rubyonra...](https://groups.google.com/forum/?fromgroups=#!topic/rubyonrails-
security/4_YvCpLzL58) (JSON library)

And in a short followup:

"To be clear, updating Rails doesn't necessarily mean the JSON gem will be
updated. Please ensure that you are running JSON version 1.7.7, 1.6.8, or
1.5.5. You can do this by adding the dependency to your Gemfile." -- Aaron
Patterson

~~~
chaz
Also a good time to remind people to subscribe to the security mailing list:

<https://groups.google.com/forum/#!forum/rubyonrails-security>

~~~
zwily
We have new emails to rubyonrails-security triggering PagerDuty alarms as
well.

~~~
tenderlove
Ouch. Sorry about that. :-(

~~~
zwily
Heh, it's okay. We just want to make sure we can patch right away. Your 5
emails at lunchtime today were fun. :)

------
sync
This one seems pretty amateur and should have been caught as part of a pull
request/code review process.

    
    
       - @regex = /^(#{Regexp.escape(@prefix)})(.+?)(#{Regexp.escape(@suffix)})$/
       + @regex = /\A(#{Regexp.escape(@prefix)})(.+?)(#{Regexp.escape(@suffix)})\z/
    

^ and $ only match the first line in ruby, whereas \A and \z match across all
lines.

~~~
tptacek
This is an extremely common bug that is not specific to Rails. It would be
worth reviewing your code to look at every regex to see if you have similar
flaws.

~~~
pjungwir
I seem to remember a blog post about this regex issue here on HN a few months
ago. It definitely surprised me to learn that Ruby doesn't treat $ as end-of-
string by default.

~~~
fxn
I blogged in <http://advogato.org/person/fxn/diary/498.html> some key
differences between Perl and Ruby regexp flags (which includes this gotcha).

------
IgorPartola
Is stuff like this vulnerability present in Django as well, and just not being
discovered as quickly, or is there something in the water (or was there years
ago) in the Rails camp that caused all these bugs?

~~~
jerf
One of the core issues here was a serialization library that was too
convenient, with the ability to deserialize into arbitrary classes and end up
executing code from them. In the Python world, this vulnerability has been a
bit better known for a while (which is not to say that they are immune, but
it's at least been common community knowledge for a very long time now, at
least a decade). Pickle is well-known to have it, but it's frequently pointed
out in the docs that it is possible and that you shouldn't unpickle things
from users. But other libraries may have this too, and if Django uses them,
have a poke around. JSON libraries would be one obvious point. There's
probably some extant Perl vulnerabilities in this area too. PHP wouldn't
surprise me (though it would be a particular framework and probably not the
language as a whole).

~~~
jey
I doubt it's an issue of Python community vs Ruby community. Instantiating
arbitrary objects is the sort of feature I'd expect an _object serialization_
library to have, but not a _document serialization_ library. Pickle is clearly
an object serialization library, and I would have naively assume that YAML is
a document serialization format, but apparently either the authors of the Ruby
YAML parser or the creators of YAML don't see it that way, and see YAML as an
object serialization format that is by default permitted to specify arbitrary
objects to instantiate.

~~~
SoftwareMaven
Python's YAML library is also capable of serializing python objects[1]. My
understanding is the YAML spec allows these extensions but does not require
them.

If you are using PyYaml.Loader instead of PyYaml.SafeLoader for anything
coming from a user, you are at risk of this problem.

[http://pyyaml.org/wiki/PyYAMLDocumentation#YAMLtagsandPython...](http://pyyaml.org/wiki/PyYAMLDocumentation#YAMLtagsandPythontypes)

------
phillmv
Shameless self promotion: we're working on something to make dealing with
these patches less painful.

It's not out yet, but soon: <http://gemcanary.com>

~~~
colinhowe
xcanary would be better... e.g. alerts on mysql, django, ubuntu... anything :)

~~~
wishbear
could you please post link to xcanary? Google doesn't helps to find it :(

~~~
voltagex_
It doesn't exist yet. Gemcanary is the MVP.

------
Tomdarkness
Surely you should be using attr_accessible with
config.active_record.whitelist_attributes = true anyway? I can't imagine a
situation where you'd want to have to manually blacklist attributes over
whitelisting them.

~~~
liquidise
How about when you want the majority of your attributes whitelisted? I
understand the urge to whitelist, but lets be reasonable here.

If i have a table with 20 columns, 19 of which i want accessible (lets exclude
a private UK). I also expect the schema for the table to be volatile. Why
should i even consider while listing 19+ over blacklisting 1?

~~~
jey
The idea is to fail in the direction of being safer than unsafe, if for
example someone adds a database column and forgets to write "attr_protected"
in the Rails code.

------
dcu
Looks like this is actually an ActiveModel vulnerability and maybe mongoid is
affected too.

Just like when rails people said they had a vulnerability in action dispatch
but it was actually a YAML vulnerability. (both used in a lot of non-rails
projects)

So please don't overlook this issue even if you are not using rails.

------
petenixey
Patio11 is indeed the security soothsayer

~~~
davidw
It didn't take much "ear to the ground" to tell that a lot of people had
started very carefully sifting through Rails code.

~~~
dasil003
The _idea_ for his post was nothing special, but the post itself was very well
written and deserves a lot of credit.

~~~
davidw
Sure, fair enough, he's a good writer.

------
saurabhnanda
I'm building a fairly large web-product for my startup in Rails, and I'm
really worried now. I'd paint the recent security vulnerabilities with one
broad stroke -- someone trusted the client too much. Isn't that something
really, really basic when dealing with data received from a remote client?
Never trust anything?

Right from the Github mass-assignment [1] vulnerability to the recent YAML &
JSON parsing vulnerabilities, it's the same core concept being violated.

This gets me thinking -- is Rails the right choice for a large project with
JSON, XML, & regular HTML endpoints?

PS: I'm not sure what the code-review policy for Rails is, but now would be
the time to call-out people who wrote this bad code and NOT auto-merge their
future commits without at least two peer reviews.

~~~
static_typed
Now is a good time to review alternative frameworks. A lot of them are simpler
to understand, rely on less magic, and have communities around them that are
interested in security as well as functionality.

~~~
donw
No, before starting to build a product is a good time to do that.

Now is a good time to patch your code and keep building your company.

Every framework has security bugs.

Jumping ship to a framework you don't understand, possibly one that is harder
to update, is a knee-jerk reactionary response to the problem.

If all these compromises worry you, invest some time in setting a HIDS (Host
Intrusion Detection System), subscribing to the relevant security mailing
lists, and ensuring that your deployment workflow allows you to patch
production code within a few minutes.

~~~
static_typed
At the end of the day, it is a trade-off: do I stick with a current framework
full of security holes, indicative of poor design and keep the daily patch
cycle fingers-crossed, or do I draw a line, migrate to a less magic less shiny
but more secure better engineered framework and focus my time on building my
apps instead of spending it all on patching. Tough call.

------
hayksaakian
If I'm not using AR (mongoid instead) and I don't have any explicit JSON.parse
in my code, am I OK?

~~~
djbender
It looks like this is specific to active_model.

[https://rubyonrails-
security.googlegroups.com/attach/bb44b98...](https://rubyonrails-
security.googlegroups.com/attach/bb44b98a73ef1a06/3-2-attr_protected.patch?view=1&part=6)

------
TallboyOne
Yikes, looks like my morning is shot now

------
daviding
Given that there is no 3.0.X update this time, where's a good resource to
learn about applying the manual patch?

~~~
steveklabnik
I have a blog post here: [http://blog.steveklabnik.com/posts/2012-10-04-run-
rails-with...](http://blog.steveklabnik.com/posts/2012-10-04-run-rails-with-
custom-patches)

~~~
daviding
Thank you. Also, info about git patch rather than a git cherrypick, in-case
it's useful to anyone:

[https://ariejan.net/2009/10/26/how-to-create-and-apply-a-
pat...](https://ariejan.net/2009/10/26/how-to-create-and-apply-a-patch-with-
git)

------
homakov
what is bypass?

~~~
tinco
it means it was circumvented, i.e. you evaded the piece of code that was
supposed to lock you out.

In this case, apparently it was possible to 'hide' your attribute behind a
newline, making it invisible to the attr_protected code, but somehow the
attribute could still be valid (for no reason rails calls #strip on it or
something?).

~~~
homakov
that's why I asked. I know attribute_assignment.rb code pretty well - no strip
is called.

So conclusion: this doesn't lead to mass assignment. only DoS.

~~~
benmmurphy
i can't actually explain why it works but it does work. I think it is result
of both of the buggy regular expressions.

~~~
homakov
i am checking agains rails 4.

[29] pry(main)> x.update_attributes("client_\nsecret"=>1) (0.1ms) begin
transaction (0.1ms) rollback transaction ActiveRecord::UnknownAttributeError:
unknown attribute: client_ secret

But DEPRECATION WARNING: The method `sdf client_secret=', matching the
attribute `client_secret' has dispatched through method_missing. This
shouldn't happen, because `client_secret' is a column of the table. If this
error has happened through normal usage of Active Record (rather than through
your own code or external libraries), please report it as a bug. (called from
block in assign_attributes at
/Users/homakov/.rvm/gems/ruby-1.9.3-p194/bundler/gems/protected_attributes-369818eedeaa/lib/active_record/mass_assignment_security/attribute_assignment.rb:67)

So it's hidden in method_missing!

------
kclay
I don't use rails but it seems that the quality of releases have been going
downhill over the pass months.

~~~
papsosouid
The quality was always terrible, these are ancient bugs that are just being
noticed now. Rails is designed with a 'convenience first, then use a couple
regexes to "secure" it' mentality. Any software designed like that will be
full of these sorts of holes.

~~~
igravious
I've spent the last 10 minutes reading your comment history. You come across
as an opinionated argumentative snide jerk. Note: before you take my comment
apart and feed it back to me realize that I have no desire in getting into a
verbal sparring match with you and won't reply to you.

I suggest that you follow the advice, "if you can't think of anything nice or
constructive to say, bite your tongue". The irony is not lost on me that in
taking you to task for the tone you couch your comments in I am failing to
live up to my own advice, but I'll make an exception here.

~~~
tanel
<http://en.wikipedia.org/wiki/Ad_hominem>

~~~
igravious
Indeed, but not quite. Ask yourself if it was my intent to counter the
person's claims by attacking the person. No, that was not my intent. I never
intended to counter the person's claims. I was explicitly calling the person
out based on tone and style of his/her comment and others in his/her comment
history.

The thought even crossed my mind while carefully drafting the above comment
that I ought to preempt the accusation that I was committing this logical
fallacy but I decided not to and now I wish the opposite.

Anyway, I hope you see the difference?

~~~
SkyMarshal
Pro-tip: People are going to have strong opinions here. Rebutting their
message rather than the delivery ends up rebutting both.

