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

Security Alerts #750

Closed
chaoscommencer opened this issue Jan 8, 2023 · 11 comments
Closed

Security Alerts #750

chaoscommencer opened this issue Jan 8, 2023 · 11 comments

Comments

@chaoscommencer
Copy link

chaoscommencer commented Jan 8, 2023

Troubleshooting

Before submitting a bug report please read the Troubleshooting doc.

Behaviour

I just forked this repository. I enabled dependabot, etc. for security-related purposes prior to use. I received the following two issues:

  1. Azure Storage Account Access Key #1: The Azure secret in dist/index.js is compromised
  2. High severity rating vulnerability for "Prototype Pollution in JSON5 via Parse Method". The suggested fix is "chore(deps): Bump json5 from 2.2.0 to 2.2.3"

Steps to reproduce this issue

  1. Enable dependabot
  2. Enable key scanning

Expected behaviour

No issues should arise from dependabot and key scanning. No keys should be compromised.

Actual behaviour

There are alerts, including one for a compromised Azure key and for a high-severity rating vulnerability.

@crazy-max
Copy link
Member

Merged #749

@chaoscommencer
Copy link
Author

You are fast good sir! Thank you for addressing in such a timely fashion!

@chaoscommencer
Copy link
Author

The key that key scanning flags appears to have been added in commit 22acf7c.
I guess you would need to rewrite the history to address the compromised warning.
Not sure if you're familiar with how to do that or if that's a practice this repository's guidelines allows its developers to do?

@crazy-max
Copy link
Member

crazy-max commented Jan 8, 2023

You are fast good sir! Thank you for addressing in such a timely fashion!

I was literally reviewing this PR and saw your issue just after merging it 😅

The key that key scanning flags appears to have been added in commit 22acf7c.

This is a pretty old commit. I don't see any security-related issue there. Do you have a report? Looks to be a false-positive to me.

@chaoscommencer
Copy link
Author

The report is simply that since the compromised key still exists in the repo history, it can be detected; and GitHub's key scanning is producing a warning about it (even though it's no longer being used). The only way to make GitHub's key scanning happy regarding this issue would be to modify the old commit and rebase the remaining commits on top of the safely modified commit. I'm guessing that's not a viable solution at this point, but thought I'd raise it just in case.

@crazy-max
Copy link
Member

compromised key

Which compromised key? I believe this is a false-positive as dist/index.js is generated code. Please provide a security report. In the meantime I close this issue. Thanks.

@chaoscommencer
Copy link
Author

chaoscommencer commented Jan 8, 2023

Hi, alternatively I can override and close the warning in my fork, but here is the security report:

Azure Storage Account Access Key
#1
Open
GitHub detected a secret 9 hours ago · Beta Give us feedback
This secret is compromised
Anyone with read access can discover secrets exposed in this repository, potentially resulting in unauthorized access to your services.
Suggested action: If this is an active Azure Storage Account Access Key, then rotate and revoke the secret to avoid any irreversible damage.
Exposed secret (I can update this comment with the secret if you'd like, but I figured it would be safer not to?)
...

Detected in 1 location

dist/index.js
@crazy-max [Test GitHub Cache 22acf7c]...

@crazy-max
Copy link
Member

@chaoscommencer Please send the report to security@docker.com. Thanks.

@chaoscommencer
Copy link
Author

Will do.

@crazy-max
Copy link
Member

Ah ok I see the report on the repo that I have dismissed in the past. The "compromised" API key is the default one for the Azurite emulator: https://learn.microsoft.com/en-us/azure/storage/common/storage-configure-connection-string#configure-a-connection-string-for-azurite. So it's ok to discard this alert.

@chaoscommencer
Copy link
Author

Ah, ok, thanks. Just wanted to make sure it was known and the key was revoked at the least.

Guess I can stop writing that email now ;). Thanks!

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

No branches or pull requests

2 participants