Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch StandardError during Base64 decoding in message encryptor. #51846

Merged
merged 1 commit into from
May 24, 2024

Conversation

simi
Copy link
Contributor

@simi simi commented May 16, 2024

Motivation / Background

RubyGems.org got recently facing 500 HTTP responses since user mangled cookies (app uses cookie store) and provided custom value which (thanks to series of edge-cases) ended up triggering NoMethodError: undefined method unpack1' for nil raised down from Base64 class. Per my understanding there is no way to catch this in application and this needs to be fixed in Rails itself.

Detail

I have tracked this issue down to ActiveSupport::Messages::Code class expecting Base64 failing with ArgumentError only. But Base64 can fail also with other exceptions when wrong input type provided. In the shared case nil is actually passed to Base64.strict_decode64 resulting into NoMethodError. I have changed rescue to handle StandardError since similar approach is used few lines bellow in deserialize method.

As an side effect, this tests this unhappy branch of decode method which wasn't covered by active support test suite before.


I have originally considered updating Base64 to raise ArgumentError when nil passed, but there are other values which can still raise similar error and it would be needed to check if passed object responds to methods needed for Base64 calculation and that's not usually happening in stdlibs.

@rafaelfranca rafaelfranca merged commit ad41f71 into rails:main May 24, 2024
3 checks passed
rafaelfranca added a commit that referenced this pull request May 24, 2024
Catch StandardError during Base64 decoding in message encryptor.
rafaelfranca added a commit that referenced this pull request May 24, 2024
Catch StandardError during Base64 decoding in message encryptor.
@simi simi deleted the base64-standard-error branch May 31, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants