ActiveRecord Vulnerability - Circumvention of attr

Re: ActiveRecord Vulnerability - Circumvention of attr_protected

#71

Post by homakov » Mon, Feb 11, 2013, 10:53 PM UTC

tinco wrote: post #44

homakov wrote: post #13

what is bypass?

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?).

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.

Re: ActiveRecord Vulnerability - Circumvention of attr_protected

#72

Post by homakov » Mon, Feb 11, 2013, 10:59 PM UTC

tptacek wrote: post #20

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…

Attribute is multiline anyway. As far as i know attribute_assignment.rb there is no .strip. So: what is profit of the "Circumvention"? Would love to know

Re: ActiveRecord Vulnerability - Circumvention of attr_protected

#73

Post by dnerdy » Mon, Feb 11, 2013, 11:02 PM UTC

IgorPartola wrote: post #29

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?

As others have said, Django may or may not have security issues. I wouldn't bet against it.

With regard to this vulnerability, however, the '^' and '$' regex pattern characters in python match the beginning and end (or end + '\n') of the string by default. Multiline mode has to be enabled explicitly:

import re

re.match(r'^test$', 'test\n multiline') == None

re.match(r'^test$', 'test\n multiline', re.MULTILINE) != None

So, I think it's a little less likely that this particular vulnerability would be an issue. It's still possible for someone to leave off the '$', but at least that case is a little more obvious.

Also, the Django codebase doesn't have any param processing code that uses whitelisting/blacklisting like this; you have to explicitly lookup values in request.GET and request.POST or use specific field names in a Form. It's a little less convenient compared to mass assignment, but more secure by default.

Re: ActiveRecord Vulnerability - Circumvention of attr_protected

#74

Post by tenderlove » Mon, Feb 11, 2013, 11:13 PM UTC

zwily wrote: post #34

chaz wrote: post #16

Earlier quoted context omitted.

Also a good time to remind people to subscribe to the security mailing list: https://groups.google.com/forum/#!forum/rubyonrails-security

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

Ouch. Sorry about that. :-(

Re: ActiveRecord Vulnerability - Circumvention of attr_protected

#75

Post by benmmurphy » Mon, Feb 11, 2013, 11:33 PM UTC

homakov wrote: post #71

tinco wrote: post #44

Earlier quoted context omitted.

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?).

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.

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

Re: ActiveRecord Vulnerability - Circumvention of attr_protected

#76

Post by tanel » Mon, Feb 11, 2013, 11:41 PM UTC

igravious wrote: post #66

papsosouid wrote: post #49

Earlier quoted context omitted.

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.

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…

http://en.wikipedia.org/wiki/Ad_hominem

Re: ActiveRecord Vulnerability - Circumvention of attr_protected

#77

Post by ubernostrum » Mon, Feb 11, 2013, 11:49 PM UTC

IgorPartola wrote: post #29

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?

Don't know, but I do know we'll be investigating it.

(we tend to keep an eye out for issues affecting other frameworks/libraries, both to coordinate and to check our own stuff -- security is really damned hard, and the thing to do is watch and learn rather than point and laugh)

Re: ActiveRecord Vulnerability - Circumvention of attr_protected

#78

Post by shabble » Tue, Feb 12, 2013, 12:05 AM UTC

dnerdy wrote: post #73

IgorPartola wrote: post #29

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?

As others have said, Django may or may not have security issues. I wouldn't bet against it. With regard to this vulnerability, however, the '^' and '$' regex pattern characters in python match the beginning and end (or end + '\n') of the string by default. Multiline mode has to be enabled explicitly: import re re.match(r'^test$', 'test\n multiline') == None re.match(r'^test$', 'test\n multiline', re.MULTILINE) != Non…

Django ModelForms[1] do seem (to my rails-ignorant self) to be quite similar to what's being described here.

    class SomeForm(ModelForm):
        class Meta:
            model = SomeModel
            fields = [ whitelist ]
            exclude = [ blacklist ]

Both fields and exclude are optional, if neither are specified 'all'[2] fields for the model will be included in the form.

[1] https://docs.djangoproject.com/en/1.4/topics/forms/modelform...

[2] The model can blacklist certain fields with editable=False in the field definition as well, which afaik trumps anything a ModelForm does.

Re: ActiveRecord Vulnerability - Circumvention of attr_protected

#79

Post by igravious » Tue, Feb 12, 2013, 12:20 AM UTC

tanel wrote: post #76

igravious wrote: post #66

Earlier quoted context omitted.

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…

http://en.wikipedia.org/wiki/Ad_hominem

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?

Re: ActiveRecord Vulnerability - Circumvention of attr_protected

#80

Post by homakov » Tue, Feb 12, 2013, 12:46 AM UTC

benmmurphy wrote: post #75

homakov wrote: post #71

Earlier quoted context omitted.

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.

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

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!