Skip to content

Commit

Permalink
Adds check for crypt module usage as weak hash (#1018)
Browse files Browse the repository at this point in the history 
The crypt module also permits creating weak hashes such as MD5
just like hashlib. This change extends the hashlib plugin to add
the extra checks on calls to crypt.crypt and crypt.mksalt which
both take a hash method parameter.

The new checks won't necessarily catch all weak hashes available
as the operating system might provide others that the crypt
module picks up. But it will capture cases with the default set.
Namely, METHOD_CRYPT", METHOD_MD5, and METHOD_BLOWFISH.

Also note that crypt.methods is supposed to return a list all
available hash methods. However, testing has shown that it can
return just METHOD_CRYPT, the weakest of the bunch. And passing
None as args to these module functions will default to the
highest available hash from crypt.methods. So this also can
be weak by default, but no reliable for Bandit to detect.

https://docs.python.org/3.11/library/crypt.html#module-crypt

Fixes #1017

Signed-off-by: Eric Brown <[email protected]>
  • Loading branch information
@ericwb
ericwb authored Apr 10, 2023
1 parent 36fc7be commit c4da1cb
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 56 deletions.
137 changes: 85 additions & 52 deletions bandit/plugins/hashlib_insecure_functions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
======================================================================
This plugin checks for the usage of the insecure MD4, MD5, or SHA1 hash
functions in ``hashlib``. The ``hashlib.new`` function provides
functions in ``hashlib`` and ``crypt``. The ``hashlib.new`` function provides
the ability to construct a new hashing object using the named algorithm. This
can be used to create insecure hash functions like MD4 and MD5 if they are
passed as algorithm names to this function.
Expand All @@ -17,6 +17,10 @@
does additional checking for usage of keyword usedforsecurity on all
function variations of hashlib.
Similar to ``hashlib``, this plugin also checks for usage of one of the
``crypt`` module's weak hashes. ``crypt`` also permits MD5 among other weak
hash variants.
:Example:
.. code-block:: none
Expand All @@ -40,6 +44,9 @@
.. versionchanged:: 1.7.3
CWE information added
.. versionchanged:: 1.7.6
Added check for the crypt module weak hashes
""" # noqa: E501
import sys

Expand All @@ -48,64 +55,90 @@
from bandit.core import test_properties as test

WEAK_HASHES = ("md4", "md5", "sha", "sha1")


def _hashlib_func(context):
if isinstance(context.call_function_name_qual, str):
qualname_list = context.call_function_name_qual.split(".")

if "hashlib" in qualname_list:
func = qualname_list[-1]
keywords = context.call_keywords

if func in WEAK_HASHES:
if keywords.get("usedforsecurity", "True") == "True":
return bandit.Issue(
severity=bandit.HIGH,
confidence=bandit.HIGH,
cwe=issue.Cwe.BROKEN_CRYPTO,
text=f"Use of weak {func.upper()} hash for security. "
"Consider usedforsecurity=False",
lineno=context.node.lineno,
)
elif func == "new":
args = context.call_args
name = args[0] if args else keywords.get("name", None)
if isinstance(name, str) and name.lower() in WEAK_HASHES:
if keywords.get("usedforsecurity", "True") == "True":
return bandit.Issue(
severity=bandit.HIGH,
confidence=bandit.HIGH,
cwe=issue.Cwe.BROKEN_CRYPTO,
text=f"Use of weak {name.upper()} hash for "
"security. Consider usedforsecurity=False",
lineno=context.node.lineno,
)


def _hashlib_new(context):
if isinstance(context.call_function_name_qual, str):
qualname_list = context.call_function_name_qual.split(".")
func = qualname_list[-1]

if "hashlib" in qualname_list and func == "new":
args = context.call_args
keywords = context.call_keywords
name = args[0] if args else keywords.get("name", None)
if isinstance(name, str) and name.lower() in WEAK_HASHES:
WEAK_CRYPT_HASHES = ("METHOD_CRYPT", "METHOD_MD5", "METHOD_BLOWFISH")


def _hashlib_func(context, func):
keywords = context.call_keywords

if func in WEAK_HASHES:
if keywords.get("usedforsecurity", "True") == "True":
return bandit.Issue(
severity=bandit.HIGH,
confidence=bandit.HIGH,
cwe=issue.Cwe.BROKEN_CRYPTO,
text=f"Use of weak {func.upper()} hash for security. "
"Consider usedforsecurity=False",
lineno=context.node.lineno,
)
elif func == "new":
args = context.call_args
name = args[0] if args else keywords.get("name", None)
if isinstance(name, str) and name.lower() in WEAK_HASHES:
if keywords.get("usedforsecurity", "True") == "True":
return bandit.Issue(
severity=bandit.MEDIUM,
severity=bandit.HIGH,
confidence=bandit.HIGH,
cwe=issue.Cwe.BROKEN_CRYPTO,
text=f"Use of insecure {name.upper()} hash function.",
text=f"Use of weak {name.upper()} hash for "
"security. Consider usedforsecurity=False",
lineno=context.node.lineno,
)


def _hashlib_new(context, func):
if func == "new":
args = context.call_args
keywords = context.call_keywords
name = args[0] if args else keywords.get("name", None)
if isinstance(name, str) and name.lower() in WEAK_HASHES:
return bandit.Issue(
severity=bandit.MEDIUM,
confidence=bandit.HIGH,
cwe=issue.Cwe.BROKEN_CRYPTO,
text=f"Use of insecure {name.upper()} hash function.",
lineno=context.node.lineno,
)


def _crypt_crypt(context, func):
args = context.call_args
keywords = context.call_keywords

if func == "crypt":
name = args[1] if len(args) > 1 else keywords.get("salt", None)
if isinstance(name, str) and name in WEAK_CRYPT_HASHES:
return bandit.Issue(
severity=bandit.MEDIUM,
confidence=bandit.HIGH,
cwe=issue.Cwe.BROKEN_CRYPTO,
text=f"Use of insecure crypt.{name.upper()} hash function.",
lineno=context.node.lineno,
)
elif func == "mksalt":
name = args[0] if args else keywords.get("method", None)
if isinstance(name, str) and name in WEAK_CRYPT_HASHES:
return bandit.Issue(
severity=bandit.MEDIUM,
confidence=bandit.HIGH,
cwe=issue.Cwe.BROKEN_CRYPTO,
text=f"Use of insecure crypt.{name.upper()} hash function.",
lineno=context.node.lineno,
)


@test.test_id("B324")
@test.checks("Call")
def hashlib(context):
if sys.version_info >= (3, 9):
return _hashlib_func(context)
else:
return _hashlib_new(context)
if isinstance(context.call_function_name_qual, str):
qualname_list = context.call_function_name_qual.split(".")
func = qualname_list[-1]

if "hashlib" in qualname_list:
if sys.version_info >= (3, 9):
return _hashlib_func(context, func)
else:
return _hashlib_new(context, func)

elif "crypt" in qualname_list and func in ("crypt", "mksalt"):
return _crypt_crypt(context, func)
15 changes: 15 additions & 0 deletions examples/crypto-md5.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from Cryptodome.Hash import MD5 as pycryptodomex_md5
from Cryptodome.Hash import SHA as pycryptodomex_sha
import hashlib
import crypt

hashlib.md5(1)
hashlib.md5(1).hexdigest()
Expand All @@ -32,3 +33,17 @@

hashes.MD5()
hashes.SHA1()

crypt.crypt("asdfasdfasdfasdf", salt=crypt.METHOD_CRYPT)
crypt.crypt("asdfasdfasdfasdf", salt=crypt.METHOD_MD5)
crypt.crypt("asdfasdfasdfasdf", salt=crypt.METHOD_BLOWFISH)
crypt.crypt("asdfasdfasdfasdf")
crypt.crypt("asdfasdfasdfasdf", salt=crypt.METHOD_SHA256)
crypt.crypt("asdfasdfasdfasdf", salt=crypt.METHOD_SHA512)

crypt.mksalt(crypt.METHOD_CRYPT)
crypt.mksalt(crypt.METHOD_MD5)
crypt.mksalt(crypt.METHOD_BLOWFISH)
crypt.mksalt()
crypt.mksalt(crypt.METHOD_SHA256)
crypt.mksalt(crypt.METHOD_SHA512)
8 changes: 4 additions & 4 deletions tests/functional/test_functional.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,29 +113,29 @@ def test_crypto_md5(self):
"SEVERITY": {
"UNDEFINED": 0,
"LOW": 0,
"MEDIUM": 10,
"MEDIUM": 16,
"HIGH": 9,
},
"CONFIDENCE": {
"UNDEFINED": 0,
"LOW": 0,
"MEDIUM": 0,
"HIGH": 19,
"HIGH": 25,
},
}
else:
expect = {
"SEVERITY": {
"UNDEFINED": 0,
"LOW": 0,
"MEDIUM": 16,
"MEDIUM": 22,
"HIGH": 4,
},
"CONFIDENCE": {
"UNDEFINED": 0,
"LOW": 0,
"MEDIUM": 0,
"HIGH": 20,
"HIGH": 26,
},
}
self.check_example("crypto-md5.py", expect)
Expand Down

0 comments on commit c4da1cb

Please sign in to comment.