
We were reviewing a settings page last night. Nothing exotic: a signed-in user changes their password, and the current password has to be typed in to confirm. Rails gives you that for free through has_secure_password, which adds a password_challenge attribute that validates against the persisted password.
The policy (we're using ActionPolicy) that filtered the params looked like this:
# with_defaults is part of the security contract, not convenience:
# has_secure_password only runs the challenge validation when the
# attribute is set at all, so an omitted password_challenge param must
# become an empty (always-failing) challenge, never a bypass.
params_filter do |params|
params.permit(:password_challenge, :password).with_defaults(password_challenge: "")
end
I like that comment. Somebody thought about the failure mode, wrote down the reasoning, and defended against it. The specs backed it up: send the request without a challenge, get a 422, password unchanged.
It was still bypassable.
nil is not an empty string
with_defaults is reverse_merge. It fills in a value when the key is missing. The whole defense rests on the attacker's request not having the key.
So don't omit it. Send it empty, in the shape only a raw body can produce:
user[password_challenge]&user[password]=attacker-password
No = after the key. Rack parses that to nil, not to "":
Rack::Utils.parse_nested_query("user[password_challenge]&user[password]=new")
# => {"user" => {"password_challenge" => nil, "password" => "new"}}
Every link in the chain is reasonable on its own:
- A key without
=parses tonil. NilClassis a permitted scalar, sopermitkeeps the key.- The key is present, so
with_defaultsdoes nothing. has_secure_passwordgates its validation on truthiness, andnilis falsy.
Step 4 is the one worth staring at. Here it is in activemodel, unchanged for years:
validate do |record|
if challenge = record.public_send(:"#{attribute}_challenge")
digest_was = record.public_send(:"#{attribute}_digest_was") if ...
unless digest_was.present? && BCrypt::Password.new(digest_was).is_password?(challenge)
record.errors.add(:"#{attribute}_challenge")
end
end
end
if challenge = .... A nil challenge doesn't fail the validation. It skips it.
The result on a real run: HTTP 302, old password dead, attacker's password live. Anyone holding a session without the password (a stolen cookie, an unattended laptop) owns the account. In our case the page also had a sign-out-everywhere step on password change, which meant the attacker locked the real owner out on the way in. The protection turned into the weapon.
Why the specs didn't catch it
This is the part I keep thinking about. There were two no-bypass tests, both green, both honest:
patch settings_password_path, params: {user: {password: "brand-new-password"}}
expect(response).to have_http_status(:unprocessable_content)
That omits the key. It tests the case with_defaults handles. The dangerous case, a present key with a nil value, is not reachable through Rack::Test's hash params at all: password_challenge: nil serializes to "" on the way out, which is the safe path. The test suite cannot express the attack unless you drop to a raw body:
patch settings_password_path,
params: "user[password_challenge]&user[password]=attacker-password",
headers: {"CONTENT_TYPE" => "application/x-www-form-urlencoded"}
So the tests weren't sloppy. They were written in a dialect that has no word for the attack.
The fix is one line
params_filter do |params|
filtered = params.permit(:password_challenge, :password)
filtered[:password_challenge] = filtered[:password_challenge].to_s
filtered
end
to_s collapses both the missing key and the nil value into "". And "" is truthy in Ruby, so the validation always runs, and always fails when it should.
Why not presence ||? Because "".presence is nil, and you are back where you started. This is the same trap wearing a different hat, and it looks tidier, which makes it worse.
Why not fix it in the controller? You can, but then the guarantee lives wherever someone remembers to put it. The params filter is the one place every request goes through.
Is this a Rails bug?
No, and that's the uncomfortable part. It's documented behavior. From the has_secure_password docs:
Additionally, a
XXX_challengeattribute is created. When set to a value other thannil, it will validate against the currently persisted password.
There it is, stated plainly. The challenge is optional by design, which is what makes it usable on forms that don't always ask for it. What the docs never say is the consequence: if an attacker can make the value nil, the check disappears. It reads as a feature, not as a warning.
So how many apps have this?
I went looking, and the news is better than I expected. Rails 8's authentication generator doesn't use password_challenge at all. Grep the generator templates and you get nothing. In the whole framework the attribute appears in exactly one file, active_model/secure_password.rb. There is no scaffold spraying this into thousands of apps.
The news is also worse than I expected. If you use it, you wired it up yourself, and the most obvious wiring is this:
params.require(:user).permit(:password_challenge, :password)
Straight into update. That is exactly as vulnerable as our version was, minus the comment claiming it was safe. We at least wrote our assumption down, which is the only reason it was falsifiable.
I don't have a number and I'm not going to guess at one. But the conditions for a long-lived vulnerability are all here: the docs describe it as a feature, the obvious implementation is wrong, and the standard test setup cannot reproduce the attack.