DEV Community

G Hewitt
G Hewitt

Posted on

I Could Hack My Own Rails App With One Hidden Input

I've been building RepBoard, a Rails app that gives freelancers a portable reputation profile — collect reviews from clients, share one professional link. It's deployed. It has accounts on it. Until this week I would have told you it was reasonably secure.

Then I read two tutorials about locking down a different app, and instead of just following along, I ran the same audit on my own code.

To my surprise, I found four holes:

1) Two kinds of users, one boolean

RepBoard has freelancers, who get a public profile and collect reviews, and clients, who log in and leave them. The difference between the two is a single boolean column on the users table called reviewable.

I use Devise for authentication. To let users edit their own account, you tell Devise which fields are safe to accept:

# app/controllers/application_controller.rb

devise_parameter_sanitizer.permit(
  :account_update,
  keys: [ :display_name, :reviewable, :bio, :slug, :avatar_url ]
)
Enter fullscreen mode Exit fullscreen mode

See it? :reviewable is in that list.

My account settings form doesn't render a field for it. Permitting a parameter has nothing to do with whether you display it. Rails accepts whatever actually arrives in the request, and that list is the only thing deciding what counts.

So: sign in as a client. Open the account settings page. Open devtools. Add one line to the form.

<input name="user[reviewable]" value="1">
Enter fullscreen mode Exit fullscreen mode

Submit it with your own password, and you're a freelancer. You now have a public profile collecting reviews on a platform whose entire selling point is trustworthy reputation.

The fix was deleting one word:

# app/controllers/application_controller.rb

devise_parameter_sanitizer.permit(
  :account_update,
  keys: [ :display_name, :bio, :avatar_url ]
)
Enter fullscreen mode Exit fullscreen mode

The fix took about ten seconds. I had been reading that list as "the fields on my edit page." But it's more like a guest list at a door, and Rails was letting in anyone whose name was on it including names I never meant to put there.

2) Where the rule belongs

Same audit, same file family. My review creation checked that the person writing the review was a client. It never checked that the person receiving it was a freelancer and the recipient's ID came from a hidden field on the form.

Edit that field and a client could review another client. Or themselves.

I'd put the original check inside a service object. That felt right at the time, but it's the wrong place, and the reason is worth internalizing:

Validation asks "is this record valid?" Authorization asks "may this user do this?"

"You can only review a freelancer" is true no matter who's asking. That makes it a data rule, and data rules go on the model, where a console session, a rake task, or a controller I haven't written yet all get stopped by it. A service object only protects the one path that goes through the service.

# app/models/review.rb

validate :reviewer_must_be_a_client
validate :reviewee_must_be_a_freelancer

private

def reviewer_must_be_a_client
  return if reviewer.blank?
  errors.add(:base, "Only clients can leave reviews") if reviewer.reviewable?
end

def reviewee_must_be_a_freelancer
  return if reviewee.blank?
  errors.add(:base, "You can only review freelancer accounts") unless reviewee.reviewable?
end
Enter fullscreen mode Exit fullscreen mode

Four lines of actual logic. They'd been missing since I built the feature.

3) It broke my own seed data

I added the validations, ran rake sample_data to reload my dev database, and it crashed immediately.

My seed file was creating every single user as a freelancer, then generating reviews between them. Freelancer reviewing freelancer, fifty times over. Data my app had never actually permitted. It just had nothing in place to say so.

The seed file wasn't wrong because I'd been careless with it. It was wrong because I'd written it against rules that lived only in my head. The first time I wrote those rules down in code, the fake data stopped making sense.

If your test fixtures can't survive your validations, one of them is lying about what your app is.

4) The one that has nothing to do with my code

While I was in there, I opened a config file I'd never read, inherited from the project template I started with.

# config/initializers/appdev_rails_settings.rb

# These relax Rails security defaults for learning purposes
Rails.application.config.action_controller.default_protect_from_forgery = false
Enter fullscreen mode Exit fullscreen mode

That's cross-site request forgery protection (CSRF) and it's turned off.

It's fine in a course sandbox, which is what it was written for. It is not fine on something I've deployed and pointed people at. And combined with the role escalation bug above, a malicious page could have flipped a logged in user's account type without them clicking anything.

A passing security scan means the scanner found nothing. It does not mean there is nothing to find. I only found it because I opened a directory I'd been treating as furniture.

Conclusion: What I'd do differently

Read the config directory on day one. Every framework has security defaults chosen by people who thought harder about it than I have, and every starter template has someone's reasons for overriding them. Those reasons are almost never my reasons.

I'd been auditing my controllers and models for months and skipping the part of the repo where the actual decisions were made.


If you're building on a bootcamp or course template, go read config/initializers/ tonight. I'd bet you find something.

Top comments (1)

Collapse
 
circuit profile image
Rahul S

The thing I'd pull out of #1 vs #2 is that they look like the same "move it to the model" lesson but they aren't, and treating them the same leaves the first one half-fixed. reviewable: true is a perfectly legal boolean, so a model validation won't reject it — nothing about the value is wrong, what's wrong is that this actor got to set that attribute at all. That's write-authorization on a field, not value validation, and the sanitizer is the only thing standing there. Moving the review rule into the model is right for #2, but #1 needs "who may assign this attribute," which a validation doesn't give you.

The reason it survived review is worth sitting with too. A permit list is an allowlist people add to, and :reviewable reads as a normal feature line in a diff — nobody clocks it as a privilege boundary. The cheap guard is a test that asserts exactly this set of params is permitted and nothing else, so the next time someone widens it the build goes red instead of the boolean going quietly live.