dev/core#5541 Prevent generation of numeric hashes #31330
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.
Overview
This code wraps the hash generation code in
CRM/Contact/BAO/Contact/Utils::generateChecksum
to ensure numeric hashes can never be created. This is to prevent failures elsewhere in the code base, notablyCRM/Mailing/Page/View::run
which uses PHP'sis_numeric()
to check whether or not it should retrieve a mailing's contents via its ID or hash when processing a request for a mailing's public view URL.Full details in issue #5541.
Before
Hashes generated by
md5(uniqid(rand(), TRUE))
can produce numeric outputs such as7571647522317445
or109002430016e903
.After
Hash generation is wrapped in a do...while loop to ensure any numeric hash is discarded and a new one generated.
Comments
This is pretty obviously quick and dirty and I'm completely happy for others to decide it should be done in a more robust manner. Seems to me that the checks in
CRM/Mailing/Page/View.php
could likely be improved too.I do wonder though how often
is_numeric()
is being used in a similar manner elsewhere in the code.