-
Notifications
You must be signed in to change notification settings - Fork 1.7k
JS: add a js/empty-password-in-configuration-file query #7632
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the qhelp has the right length. It would be too patronising to add examples for this kind of security issue.
javascript/ql/src/Security/CWE-862/EmptyPasswordInConfigurationFile.qhelp
Outdated
Show resolved
Hide resolved
Co-authored-by: Esben Sparre Andreasen <esbena@github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Text generally looks okay but there seem to be some inconsistencies, which I've commented on.
javascript/ql/src/Security/CWE-862/EmptyPasswordInConfigurationFile.ql
Outdated
Show resolved
Hide resolved
|
||
</overview> | ||
<recommendation> | ||
<p>Choose a proper password and encrypt it if it has to stored in a configuration file.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth including the alternative possibility of storing the password in a secure store and referring to it in the configuration file?
Also - "proper" seems like a strange choice of word. Would "strong" be better here?
|
||
</overview> | ||
<recommendation> | ||
<p>Choose a proper password and encrypt it if it has to stored in a configuration file.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<p>Choose a proper password and encrypt it if it has to stored in a configuration file.</p> | |
<p>Choose a proper password and encrypt it if it has to be stored in a configuration file.</p> |
Co-authored-by: Felicity Chapman <felicitymay@github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates
Evaluation looks clean.
Inspired by this C# query.
The implementation mostly reuses the logic from our existing
PasswordInConfigurationFile.ql
query.