From c4da1cb430a905b3c9e0b77f0ee051bc9f6c2d15 Mon Sep 17 00:00:00 2001 From: Eric Brown Date: Mon, 10 Apr 2023 09:55:45 -0700 Subject: [PATCH] Adds check for crypt module usage as weak hash (#1018) 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 --- bandit/plugins/hashlib_insecure_functions.py | 137 ++++++++++++------- examples/crypto-md5.py | 15 ++ tests/functional/test_functional.py | 8 +- 3 files changed, 104 insertions(+), 56 deletions(-) diff --git a/bandit/plugins/hashlib_insecure_functions.py b/bandit/plugins/hashlib_insecure_functions.py index 15603e9b1..710800a72 100644 --- a/bandit/plugins/hashlib_insecure_functions.py +++ b/bandit/plugins/hashlib_insecure_functions.py @@ -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. @@ -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 @@ -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 @@ -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) diff --git a/examples/crypto-md5.py b/examples/crypto-md5.py index b78fd4c82..b827c707d 100644 --- a/examples/crypto-md5.py +++ b/examples/crypto-md5.py @@ -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() @@ -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) diff --git a/tests/functional/test_functional.py b/tests/functional/test_functional.py index bc8689396..9f52e910e 100644 --- a/tests/functional/test_functional.py +++ b/tests/functional/test_functional.py @@ -113,14 +113,14 @@ 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: @@ -128,14 +128,14 @@ def test_crypto_md5(self): "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)