Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change an existing rule to find more candy. #155

Merged
merged 1 commit into from
Oct 15, 2024
Merged

Conversation

zh54321
Copy link
Contributor

@zh54321 zh54321 commented Oct 11, 2024

TL;DR
Change rule for more candy.

Long story
In engagements, we encounter more and more hybrid environments (especially Azure).
Therefore, I was wondering if Snaffler identifies secrets in Azure/Entra related scripts.
Therefore, I tested several authentication methods with several Azure related tool (Azure CLI, Azure PowerShell, MS Graph, Exchange Online PNP, native API calls etc.).

Snaffler catches all scripts for tools which require a PS password object 😄 .
However, it does not catch the secret in some cases 😢 .

Sometimes customer do not use PowerShell module but native API call (example with curl). The string client_secret is currently not detected if the secret is not quoted.

The current rule (client_secret\s*=\s*[\'\"][^\\'\\\"]....) in the file KeepPassOrKeyInCode.toml catches stuff like:
client_secret = 'SuperPassword1!'
client_secret = "SuperPassword1!"

But it will miss client secrets which are not quoted:
client_secret = SuperPassword1!
client_secret = $MyPW

At least using the example by Microsoft (using CURL) the passwords are not quoted:
References: https://learn.microsoft.com/en-us/graph/auth-v2-user?tabs=curl#step-2-request-an-access-token

Therefore, I suggest to change the current regex to catch all those cases:
Old: client_secret\s*=\s*[\'\"][^\\'\\\"]....
New: client_secret\s*=\s

Using this changed rule, all credentials (quoted and unquoted) are identified:
2
(Parsed Snaffler output file)

Changing the rule identifying client secrets to identify unquoted secrets as well.
@l0ss l0ss merged commit 36903dc into SnaffCon:master Oct 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants