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

fix(secret): add prefix to exclude 0-9a-zA-Z before secret #7176

Closed
DmitriyLewen opened this issue Jul 17, 2024 · 8 comments · Fixed by #7182
Closed

fix(secret): add prefix to exclude 0-9a-zA-Z before secret #7176

DmitriyLewen opened this issue Jul 17, 2024 · 8 comments · Fixed by #7182
Assignees
Labels
scan/secret Issues relating to secret scanning
Milestone

Comments

@DmitriyLewen
Copy link
Contributor

DmitriyLewen commented Jul 17, 2024

Description

We don't need to detect secrets containing 0-9a-zA-Z before secret.
e.g. finding a secret in #define DISPID_ICANVASRENDERINGCONTEXT2D_CANVAS DISPID_CANVASRENDERCONTEXT2D is false positive.

Important points:

  • we need to check starting line/file.
  • we shouldn't check this prefix for custom secrets
@DmitriyLewen DmitriyLewen added the scan/secret Issues relating to secret scanning label Jul 17, 2024
@DmitriyLewen DmitriyLewen self-assigned this Jul 17, 2024
@knqyf263 knqyf263 added this to the v0.55.0 milestone Jul 17, 2024
@knqyf263 knqyf263 changed the title refactor(secret): add prefix to exclude 0-9a-zA-Z before secret fix(secret): add prefix to exclude 0-9a-zA-Z before secret Jul 17, 2024
@knqyf263
Copy link
Collaborator

knqyf263 commented Jul 17, 2024

@DmitriyLewen I would assign it to @afdesk. Could you assist him as you know more context?

@afdesk
Copy link
Contributor

afdesk commented Jul 18, 2024

@knqyf263 @DmitriyLewen I have a question.

Should we detect next string as a secret?

MyAWS_secret_KEY="12ASD34qwe56CXZ78tyH10Tna543VBokN85RHCas"

IMHO, yes, because it still contains sensitive data, but now Trivy skips it.
Wdyt?
thanks!

@knqyf263
Copy link
Collaborator

Which rule? Why isn't it detected now?

@afdesk
Copy link
Contributor

afdesk commented Jul 18, 2024

Only for this rule was added startSecret. so MyAWS_secret_KEY is ignored.

Regex: MustCompile(fmt.Sprintf(`(?i)%s%s%s(sec(ret)?)?_?(access)?_?key%s%s%s(?P<secret>[A-Za-z0-9\/\+=]{40})%s%s`, startSecret, quote, aws, quote, connect, quote, quote, endSecret)),

@afdesk
Copy link
Contributor

afdesk commented Jul 18, 2024

@knqyf263 is it a mistake?

@knqyf263
Copy link
Collaborator

It's weird. For me, it looks like It should be detected. @DmitriyLewen updated the regex last time. We can wait for him.
#5647

@afdesk
Copy link
Contributor

afdesk commented Jul 18, 2024

If I understand correctly, #5647 didn't affect on startSecret,
@DmitriyLewen right?

@knqyf263 thanks. I understood your point

@DmitriyLewen
Copy link
Contributor Author

DmitriyLewen commented Jul 19, 2024

I added this in aquasecurity/fanal#514
But unfortunately, 2 years later, I don't remember the reason for this (and I don't see this problem in a related question).

Looks like it was my mistake.
It would be more logical to add startSecret for aws-access-key-id instead of aws-secret-access-key.

UPD:
I'm not sure if we need to check prefix for AWS_SECRET_KEY.
UUIC aws uses a list of standard envs - https://docs.aws.amazon.com/sdk-for-php/v3/developer-guide/guide_credentials_environment.html

@knqyf263 knqyf263 modified the milestones: v0.55.0, v0.54.0 Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scan/secret Issues relating to secret scanning
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants