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

Check value of usedforsecurity for hashlib #798

Merged
merged 1 commit into from Feb 7, 2022
Merged

Conversation

ericwb
Copy link
Member

@ericwb ericwb commented Feb 6, 2022

In Python 3.9+ hashlib has a new argument named usedforsecurity
to indicate whether the hash is intended to be used for security
or not. The default value is True. So a user must explicit set
to False to state their non-security use.

As a result of this chnage in Python, the severity has been
moved up to HIGH if the usedforsecurity is True. But on earlier
versions of Python, the severity will remain at MEDIUM since
we don't know the intent of usage.

https://docs.python.org/3/library/hashlib.html#hashlib.new

Closes #748

Signed-off-by: Eric Brown browne@vmware.com

@@ -34,6 +34,10 @@
CWE information added
"""
import sys

import distutils
Copy link
Member

Choose a reason for hiding this comment

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

distutils is such a nightmare I would rather we implement strtobool ourselves instead of relying on it. I believe it is slated to be removed from the stdlib soon too so we will be broken once it does

Copy link
Member

@sigmavirus24 sigmavirus24 left a comment

Choose a reason for hiding this comment

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

Please ditch distutils

In Python 3.9+ hashlib has a new argument named usedforsecurity
to indicate whether the hash is intended to be used for security
or not. The default value is True. So a user must explicit set
to False to state their non-security use.

As a result of this chnage in Python, the severity has been
moved up to HIGH if the usedforsecurity is True. But on earlier
versions of Python, the severity will remain at MEDIUM since
we don't know the intent of usage.

https://docs.python.org/3/library/hashlib.html#hashlib.new

Closes #748

Signed-off-by: Eric Brown <browne@vmware.com>
@ericwb
Copy link
Member Author

ericwb commented Feb 6, 2022

This PR may not be complete, as I notice that usedforsecurity can also be passed to convenience functions such as
hashlib.md5(usedforsecurity=True). The problem is that those checks are currently part of the blacklist calls which
don't support more advanced checking of arguments. Should I move the hashlib blacklist calls over to the same
plugin? That might be a breaking change to backwards compatibility.

@sigmavirus24
Copy link
Member

Should I move the hashlib blacklist calls over to the same
plugin? That might be a breaking change to backwards compatibility.

Maybe in a separate PR?

@ericwb ericwb merged commit 6b6b896 into master Feb 7, 2022
@ericwb ericwb deleted the usedforsecurity branch February 7, 2022 04:17
@ericwb ericwb added this to the Release 1.7.3 milestone Feb 16, 2022
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

Successfully merging this pull request may close these issues.

Lower the severity of insecure hash functions (B303) for hashlib if usedforsecurity=False
2 participants