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

Go: Consider more strings as hardcoded credentials #15924

Merged

Conversation

atorralba
Copy link
Contributor

@atorralba atorralba commented Mar 14, 2024

Using the heuristic for dummy passwords to discard sources in this query makes us miss sources like this one, so I think it's a good idea to remove the limitation.

@github-actions github-actions bot added the Go label Mar 14, 2024
@atorralba atorralba marked this pull request as ready for review March 14, 2024 10:57
@atorralba atorralba requested a review from a team as a code owner March 14, 2024 10:57
Comment on lines -63 to +64
type = SensitiveExpr::password() and
message = "Hard-coded credential."
type = SensitiveExpr::secret() and
message = "Hard-coded $@."
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also changed this so that the alert contains a link to the source (i.e. the hard-coded string).

@owen-mc
Copy link
Contributor

owen-mc commented Mar 14, 2024

The source you linked to looks a lot like a dummy password...

@atorralba
Copy link
Contributor Author

The source you linked to looks a lot like a dummy password...

Yes. There's where the vulnerability is: that dummy password was used in production and attackers could forge JWT tokens because it was known (see CVE-2023-22463).

The dummy password heuristic makes sense for things like go/clear-text-logging, because even if such a password ends up in the logs, it's not a big deal (nothing sensitive was leaked after all if it's dummy, and users are intended to change it).

But when dealing with hard-coded passwords, even if they're dummy, they're still used for sensitive things like authentication or cryptographic operations. Even an empty string would be a problem.

Copy link
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. It all looks good. And thanks for improving the alert message in passing.

@atorralba atorralba merged commit ee3efba into github:main Mar 14, 2024
16 checks passed
@atorralba atorralba deleted the atorralba/go/hardcoded-credentials-fix branch March 14, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants