fix(azuresastoken): match SAS tokens regardless of parameter order#5043
Open
genisis0x wants to merge 1 commit into
Open
fix(azuresastoken): match SAS tokens regardless of parameter order#5043genisis0x wants to merge 1 commit into
genisis0x wants to merge 1 commit 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}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit a245faa. Configure here.
| spValuePat = regexp.MustCompile(`^[racwdli]+$`) | ||
| svValuePat = regexp.MustCompile(`^\d{4}-\d{2}-\d{2}$`) | ||
| srValuePat = regexp.MustCompile(`^[bcfso]$`) | ||
| timeValuePat = regexp.MustCompile(`^\d{4}-\d{2}-\d{2}T\d{2}(?::|%3A)\d{2}(?::|%3A)\d{2}Z$`) |
Contributor
There was a problem hiding this comment.
timeValuePat rejects valid lowercase percent-encoded timestamps
Low Severity
The timeValuePat regex only matches uppercase %3A for URL-encoded colons in timestamps, but per RFC 3986, %3a (lowercase hex) is an equally valid encoding. A SAS token with st=2025-06-18T08%3a45%3a11Z (lowercase hex digits) would fail validation and be silently missed, despite being a legitimate URL-encoded timestamp. The PR claims tolerance of URL-encoded values, but this pattern is case-sensitive on the hex digits.
Reviewed by Cursor Bugbot for commit a245faa. Configure here.
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
2 participants
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.
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-detection heuristics in one detector; verification behavior is unchanged and existing negative tests are preserved.
Overview
The Azure SAS token detector no longer depends on query parameters appearing in a fixed order or on literal
:in timestamps. It now finds contiguouskey=value&…runs with a looser regex and confirms SAS shape inkeyMatchIsSASToken(sp,sv,sr,sig, at least one ofst/se, optionalsip), including URL-encoded time and signature values.This closes gaps for tokens from tools like Azure Storage Explorer (e.g.
svfirst,%3Ain times) while keeping the same field-level checks the old regex enforced. Tests add a Storage-Explorer-style positive case and assert generic blob API query strings (e.g.comp=list) are not treated as SAS.Reviewed by Cursor Bugbot for commit a245faa. Bugbot is set up for automated code reviews on this repo. Configure here.