fix(azuresastoken): match SAS tokens regardless of parameter order#5043
fix(azuresastoken): match SAS tokens regardless of parameter order#5043genisis0x wants to merge 2 commits into
Conversation
The keyPat regex required a fixed parameter order (sp, st, se, [sip], [spr], sv, sr, sig) and literal ':' in the timestamps. Real SAS tokens vary: Azure Storage Explorer emits the parameters in a different order (sv first, sp last) and URL-encodes the ':' in st/se as %3A, so those tokens were silently missed. Match a contiguous run of query parameters and validate the SAS-specific parameters (sp, sv, sr, sig, and st/se, plus optional sip) in code instead. This makes detection order-independent and tolerant of URL-encoded values while preserving the previous validations (permission set, resource type, timestamp shape, IP format) so the existing false-positive cases still return no result. Adds test cases for the alternate-order / URL-encoded-timestamp token and for a non-SAS query string. Closes trufflesecurity#4732
|
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…amps timeValuePat only matched uppercase %3A for the URL-encoded ':' in start and expiry timestamps, but percent-encoding hex digits are case-insensitive per RFC 3986, so a token encoded with %3a was silently missed. Accept %3[Aa] in both separator positions and add a lowercase-hex test case.
|
Addressed the Bugbot note in b718eca: |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit b718eca. Configure here.
| // signature as %2B, %2F, %3D). Rather than enumerate every permutation in one | ||
| // regex, match a contiguous run of query parameters and validate the SAS-specific | ||
| // parameters in keyMatchIsSASToken below. This keeps detection order-independent. | ||
| sasQueryPat = regexp.MustCompile(`[a-z]{2,4}=[^&\s"'<>]+(?:&[a-z]{2,4}=[^&\s"'<>]+)+`) |
There was a problem hiding this comment.
Regex absorbs first SAS param when preceded by lowercase assignment
Medium Severity
The sasQueryPat value character class [^&\s"'<>]+ does not exclude =, so when a SAS token is preceded by a lowercase 2–4 character variable name and = (e.g., sas=sp=r&st=... or sas_token=sp=r&... where oken= matches), the regex greedily matches from that preceding key, absorbing the first SAS parameter as a "value." Since FindAllString returns non-overlapping matches, the correct match starting at sp is consumed and never tried independently. keyMatchIsSASToken then fails because sp is missing from the parsed params map, causing the token to be silently missed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b718eca. Configure here.
|
Hi @mariduv, thanks again for merging #5041. Whenever you have a spare moment, would appreciate a look at this one, no rush. It closes #4732: Storage-Explorer SAS tokens were missed because the matcher required a fixed parameter order and literal-colon timestamps. The fix validates the SAS params order-independently and tolerates URL-encoded timestamps. CLA is signed and all checks are green. Happy to adjust anything. |
2 participants
Description:
The
azuresastokendetector only matched SAS tokens whose query parameters appeared in one fixed order (sp,st,se,[sip],[spr],sv,sr,sig) and whose timestamps used a literal:.Real SAS tokens vary. Azure Storage Explorer, for example, emits the parameters in a different order (
svfirst,splast) and URL-encodes the:inst/seas%3A:Tokens like this were silently missed.
Instead of encoding one parameter order in a single regex, the detector now matches a contiguous run of query parameters and validates the SAS-specific parameters (
sp,sv,sr,sig, and at least one ofst/se, plus an optionalsip) in code. This makes detection order-independent and tolerant of URL-encoded values, while preserving the validations the previous regex enforced (permission set, resource type, timestamp shape, and IP format) so the existing false-positive cases still produce no result.Closes #4732
Tests:
TestAzureSASToken_Patterncases still pass.Checklist:
make test-community)?make lint)?Note
Low Risk
Scoped to secret-pattern matching in one detector; validation intent is preserved with broader true-positive coverage and explicit non-SAS filtering in tests.
Overview
The Azure SAS token detector no longer requires query parameters in a fixed order or literal
:in timestamps. It replaces the single order-locked regex with a broad match on contiguouskey=valueruns, thenkeyMatchIsSASTokenparses pairs and checks required SAS fields (sp,sv,sr,sig, at least one ofst/se, optionalsip) with the same shape rules as before, including URL-encoded timestamps (%3A/%3a) and signatures.Tests add Storage Explorer–style tokens (reordered params + encoded times), lowercase hex in encoding, and a case where a normal blob API query string must not be treated as a SAS token.
Reviewed by Cursor Bugbot for commit b718eca. Bugbot is set up for automated code reviews on this repo. Configure here.