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

False Positive for 'Password in URL' regex #216

Open
mrubino-godaddy opened this issue Sep 7, 2021 · 4 comments
Open

False Positive for 'Password in URL' regex #216

mrubino-godaddy opened this issue Sep 7, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@mrubino-godaddy
Copy link

🐛 Bug Report

The regex for the Password in URL produces false positives when attempting to externalize the password.

To Reproduce

Have a file with the given text: (in this case a docker file or github actions file. The password is externalized into a variable and then injected into the url.

+git config --global \
+    url."https://${GITHUB_USER}:${GITHUB_TOKEN}@github.com/godaddy".insteadOf \
+    "https://github.com/godaddy"

or

git config --global \
+    url."https://${GHEC_TOKEN}:[email protected]/godaddy".insteadOf \
+    "https://github.com/godaddy"
Reason: Regular Expression Match
Detail: Password in URL

Expected Behavior

The tartufo scan should pass and allow for externalize/parameterized URL auth.

Environment

$ tartufo --version
tartufo, version 2.7.1
@mrubino-godaddy mrubino-godaddy added the bug Something isn't working label Sep 7, 2021
@mrubino-godaddy
Copy link
Author

I would guess something like this:

https://user:[email protected]/godaddy          # hit
https://u$er:[email protected]/godaddy          # hit
https://user:[email protected]/godaddy          # hit
https://u$er:[email protected]/godaddy          # hit
https://${user}:[email protected]/godaddy       # hit
https://user:${password}@github.com/godaddy       # hit
https://${user}:${password}@github.com/godaddy    # safe

https://token:[email protected]/godaddy    # hit
https://to$ken:[email protected]/godaddy   # hit
https://${token}:[email protected]/godaddy # safe

@mrubino-godaddy
Copy link
Author

FWIW I tried fiddling and came to something like this for a partial (though still imperfect) improvement.

(?!\$\{\w+?\})[^\/\s@]{3,20}

@tarkatronic
Copy link
Contributor

Hi @mrubino-godaddy, thanks for reporting this! Is there any chance you would be able to take a stab at a PR for an improved regex for this purpose? The regexes are now all contained within the tartufo codebase, found here: https://github.com/godaddy/tartufo/blob/main/tartufo/data/default_regexes.json#L29

@pmevzek-godaddy
Copy link

The regex might need to be aligned to the standard definition of an URL, see RFC3986 §3.2
It says, summarizing:

authority   = [ userinfo "@" ] host [ ":" port ]
userinfo    = *( unreserved / pct-encoded / sub-delims / ":" )

where it was previously defined that:

unreserved  = ALPHA / DIGIT / "-" / "." / "_" / "~"
pct-encoded = "%" HEXDIG HEXDIG
sub-delims  = "!" / "$" / "&" / "'" / "(" / ")"
                  / "*" / "+" / "," / ";" / "="

so from all of that, in theory, https://${user}:[email protected]/godaddy is not a valid URL because { and } should be really encoded as %7B and %7D. $ is allowed though as is (because in sub-delims).

Even better: don't use regexps to parse URLs (it is tricky). Use a library that will do it properly. From it, it will extract the components, including the userinfo and then you can find that piece of data to some code that will find out if there is a secret inside it or not.
For example, as heuristics, take everything looking like http:// or https:// until like the first space (but handling cases where the URL is itself contained in quotes, or in the recommended forms of <https://...> or <URL:https://...>) and feeding that to an URL parsing library before extracting the userinfo part, or other parts too as a secret could be as well in the path, like https://www.example.com?username=bob&password=eSool9veish5aiT3o

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants