phc: add_b64_bytes keeps the ParamsString invariants - #2462
Open
yuxi-liu-wired wants to merge 1 commit into
Open
yuxi-liu-wired wants to merge 1 commit into
yuxi-liu-wired wants to merge 1 commit into
Conversation
ParamsString is meant to always hold a valid parameter string
(INVARIANT_VIOLATED_MSG, used by iter()). add_b64_bytes wrote straight
into the buffer instead of going through add(), so it:
- did not reject a duplicate name ("m=1,m=eA"), unlike add_decimal and
add_str;
- accepted values longer than Value::MAX_LENGTH (64): 49..=88 bytes
encode to 66..=118 characters, which FromStr rejects ("parameter
value too long"), so a PasswordHash built with such a param does not
parse back;
- on a Base64 length error, left the delimiter and "name=" behind.
Encode into a Value::MAX_LENGTH scratch buffer and add the result
through add(), like the other add_* methods.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
ParamsStringis meant to always hold a valid parameter string:iter()expects it withINVARIANT_VIOLATED_MSG, andFromStrvalidates every name and value.add_decimalandadd_strgo throughadd(), which rejects duplicates and leaves the buffer unchanged on overflow.add_b64_byteswrites straight into the buffer instead, and that breaks the invariant three ways:End to end, a
PasswordHashwhose params were built withadd_b64_bytesand 49–88 bytes of data (Argon2'sdata/keyidparams are B64) serializes to a PHC string thatPasswordHash::newrejects with "parameter value too long".The fix encodes into a
Value::MAX_LENGTHscratch buffer, validates the result as aValue, and adds it throughadd(), like the otheradd_*methods. Existing output is unchanged (a=AQ,b=AgM,c=BAUG).Test:
add_b64_bytes_invariantsinparams.rscovers the duplicate name, the 64-char boundary (48 bytes ok, 49 bytesParamValueTooLongwith nothing written), and an overflow of the total length that leaves the params unchanged. It fails on master. Thephc.ymlmatrix passed locally: powerset tests on stable and 1.85, thumbv7em/wasm32 powerset build, fmt, clippy.This PR was produced by AI agents (Claude) and reviewed by me before submission.