RelyingParty.verify_registration does not validate response shape

[sfnelson]

Following the examples, our app uses verify_registration(response, challenge) with untrusted inputs for response.

In production, we've seen users submit data such as this:

{"id":"...","rawId":"...","response":{"clientDataJSON":{},"attestationObject":{}},"authenticatorAttachment":"platform","clientExtensionsResults":{},"type":"public-key"}

While this is obviously invalid, the library attempted to decrypt {} as an attestationObject, failing with undefined method 'end_with?' for an instance of Hash. This caused a 500 inside critical code.

After reviewing the chain that leads to the Base64UrlEncoder.decrypt call I'm not clear on where the responsibility for sanitising user inputs should be. Is it expected that library callers check that the response conforms to an expected shape?

[santiagorodriguez96]

Hi @sfnelson! 👋

Thanks for the detailed report, and sorry for the delayed response.

I'm not clear on where the responsibility for sanitising user inputs should be. Is it expected that library callers check that the response conforms to an expected shape?

That's a good call actually – and to be honest, I don't know the correct answer either. I think that, as this input is untrusted by definition and the gem knows the expected shape of it, it could be argued that it's the gem's job to reject malformed input – in fact, we are actually already doing it in a few spots (missing client data, challenge type, token binding shape).

The catch is that doing this correctly, cleanly and consistently everywhere would involve a considerable effort and will require some thought. So I'd say it's something we might want to address, but it'll take us a bit of time to tackle properly.

In the meantime, validating that those fields are present Strings before calling verify_registration (same for the other methods from our API that receive a credential from the params) will keep these payloads out of the gem and thus prevent it from blowing up.