Comment by losvedir | Hacker News Reader

Comment by losvedir | Hacker News Reader

"This is easy to do: simply call ActiveRecord::Base.attr_accessible(nil) in an initializer, and all Rails models will automatically have mass assignment disabled until they have it explicitly enabled by attr_accessible."

I may be way wrong here, but: didn't this particular suggestion come from an HN comment? I seem to remember when there was a flurry of Diaspora articles, that a comment mentioned this as a good example of Ruby monkey patching, and one of my "heroes" (so, patio11, or maybe tpatacek?) acknowledged it as a simple and beautiful solution.

Hmm... off to see if I can find what I'm thinking of.

edit: oh, ha! Found the link: http://news.ycombinator.com/item?id=1718422 . I was sort of right, sort of wrong. It was an HN comment, but it was actually patio11's own comment to tpatacek, who thought it was a great idea!

The approach has been around for a years (see my comment in that thread, for example), but very few of us use it.

The thing you'll run into with that approach is that no authors of gems and plugins that include AR models declare attributes as accessible, effectively assuming that mass assignment is enabled and resulting in the gem/plugin not saving properly. I've had to fork and submit patches for every hugely popular gem with AR models I've used, so it appears practically no one else follows this approach in production apps.

I strongly suspect that leaving all attributes mass assignable on all models other than generated user models is extremely common in Rails apps and it's very likely that this vulnerability is ubiquitous.

Yep, this indeed caused some "fun" issues with Appointment Reminder. Off the top of my head, it broke DelayedJob and one other gem that I am forgetting.

Edited to add: Now I remember. The second gem was actually A/Bingo (a plugin). I fixed this back in January.

Yeah, DJ is actually the only one I'm still using via a fork (https://github.com/alphabetum/delayed_job/commit/9637a4418ad...). I haven't had time to put the change in a topic branch and do a pull request.

Reply on news.ycombinator.com