Ruby on Rails: Changeable model ids on vanilla update can lead to severely bad side-effects

2015-05-20T19:16:40
ID H1:63131
Type hackerone
Reporter zachaysan
Modified 2016-02-12T18:05:34

Description

Proof of concept

User.find(1).update!(id: 1701)

> But strong params!

Disagree. There are cases where less experienced users will allow "id" as a param for a subnested resource and then copy and past that code into that subnested resource's own controller, forgetting to remove the "id".

I consider this bug extremely severe since it is a very common pattern in most Rails JSON APIs and by setting the id to the maximum value may lead to the database to refuse to create new records OR in better databases like Postgres, an attacker can continue to stay just ahead of of the id sequence which will issue this error:

DETAIL: Key (id)=(1701) already exists.

This effectively stops any new content from being created with a very small number of attackers.

I have also tried to think of a way that a user could gain access to information with this, but I think that it would have to be in a very weird way. For example a multiple associations table where both tables save their associations to each other then one of them gets deleted. Too unlikely to be a major concern.

Proposed solution:

Make a special flag or special method for updating a model with a model id, but by default ignore "id" as a param. The reason I say "ignore" and not "raise exception" is that it is more convenient to not have to strip ids out of everything.