ruby-advisory-db icon indicating copy to clipboard operation
ruby-advisory-db copied to clipboard

Figure out what OSVDB-96396 (activemodel vuln) is referring to

Open reedloden opened this issue 9 years ago • 2 comments

http://osvdb.org/show/osvdb/96396 talks about a vulnerability in activemodel, but I can't figure out what commit it's referring to or when said vulnerability was fixed (if at all). Could use some help so I can get it added to the db.

reedloden avatar Jul 20 '15 10:07 reedloden

sigh I just spent 30 minutes reading through activemodel commit logs.

... no idea.

mveytsman avatar Jul 28 '15 01:07 mveytsman

I heard back from the OSVDB folk, and they said the entry was created based off of https://github.com/rails/rails/pull/8122 and https://github.com/rails/rails/commit/e8a50aa1c1f9d04c21b54e983f9a090d4b42c8eb.

reedloden avatar Jul 28 '15 02:07 reedloden

Ping. Is this lost to the sands of time?

postmodern avatar May 23 '23 20:05 postmodern

I saw that this just got pushed to the advisory DB (and is now making CI of all my Rails projects fail), but I don't see any open issue on the rails repository or any evidence that they're aware of this purported issue? Is this actually a real thing?

Roguelazer avatar May 31 '23 00:05 Roguelazer

@Roguelazer I'm getting this too and opened #612. Sorry, should have checked for searched for an already existing issue.

jmadkins avatar May 31 '23 00:05 jmadkins

I saw that this just got pushed to the advisory DB (and is now making CI of all my Rails projects fail), but I don't see any open issue on the rails repository or any evidence that they're aware of this purported issue? Is this actually a real thing?

Digging through the history, it looks like there was a follow up to https://github.com/rails/rails/pull/8122 with https://github.com/rails/rails/pull/8123, but that was also closed without being merged in. The last post in the PR mentioned that it was a breaking change, so they might need to wait for rails 5 and assess it again then

dlim87 avatar May 31 '23 01:05 dlim87

I reverted the commit because it didn't have a patched_versions (should have caught that, sorry).

postmodern avatar May 31 '23 01:05 postmodern

FWIW, It looks like this change in 5.2 might have fixed it?

https://github.com/rails/rails/blob/74b8a3798b41833e94ce1abba6d32abdc1a1dd7f/guides/source/5_2_release_notes.md?plain=1#L640

Wait no, I misread it. That PR formalizes that the intended behavior was the opposite of what was being done in https://github.com/rails/rails/pull/8122

dlim87 avatar May 31 '23 01:05 dlim87

I wrote up a repro script to test if this "vulnerability" still exists. Correct me if I'm wrong, but I interpreted the description of the advisory to mean when User.new(..., email: "...", email_confirmation: nil) is validated, that it should fail?

Don't allow confirmation to pass if confirmation value is nil and doesn't match value.

However, the title of the Rails PR mentions fixing when the _confirmation value is false.

Execute ConfirmationValidator validation when _confirmation's value is false

My repo script shows that validations will pass if the _confirmation value is nil, but will fail is the _confirmation value is false. Does this confirm that this is still an issue? If so, is this really a vulnerability or a regular bug?

postmodern avatar May 31 '23 03:05 postmodern

I'm going to call it, this probably isn't even a vulnerability. I can't think of a scenario where this behavior could have been exploited. Confirmation validation is typically used for passwords when creating an account, or entering your email when signing up for a mailing list. Bypassing confirmation validations would mean maybe you enter in the wrong password or signup to a mailing list with a typoed email address. Most likely someone was reviewing the Rails ChangeLog and thought this was a vulnerability they were quietly trying to fix without disclosing it and reported it to OSVDB.

postmodern avatar May 31 '23 23:05 postmodern