Add a separator to the cache key for ActiveSupport::CachingKeyGenerator #39143
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The ActiveSupport::CachingKeyGenerator is a thin wrapper around ActiveSupport::KeyGenerator that uses a Hash to memoize the output. The Hash key is currently a simple concatenation of the salt and the key length. This means if
generate_key
is called like@caching_generator.generate_key("133", 7)
and then@caching_generator.generate_key("13", 37)
, the cache key will be the same (1337
) and the output in the second case will be a 7-byte key, rather than a 37-byte key. Neither of these values should be user-controlled in normal usage of this class, so it is unlikely that a cache key collision here would be exploitable in practice.We fix this issue by adding a separator (
|
) that delineates the key size from the salt.Other Information
This PR was a result of a report to the Rails bug bounty program by https://hackerone.com/mysterican.