Merge pull request #91 from blacklanternsecurity/expressjs_parsing_bug

Express.js parsing /error handling bug
blacklanternsecurity · Jul 21, 2023 · 1082317 · 1082317
2 parents 2ee0fe7 + 0947d23
commit 1082317
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 10 deletions.
diff --git a/badsecrets/modules/express_signedcookies_es.py b/badsecrets/modules/express_signedcookies_es.py
@@ -35,15 +35,17 @@ def expressVerify_es(self, value, secret):
         payload, signature = value.split(".")[0][4:], urllib.parse.unquote(value.split(".")[1])
 
         with suppress(binascii.Error):
-            for hash_algorithm_str in self.search_dict(
-                self.hash_sizes, len(no_padding_urlsafe_base64_decode(signature))
-            ):
-                hash_algorithm = self.hash_algs[hash_algorithm_str]
-                generated_hash = self.expressHMAC(payload, secret, hash_algorithm)
-                if generated_hash == signature:
-                    return {
-                        "hash algorithm": hash_algorithm.__name__.split("openssl_")[1],
-                    }
+            signature_candidates = self.search_dict(self.hash_sizes, len(no_padding_urlsafe_base64_decode(signature)))
+            if not signature_candidates:
+                return False
+            else:
+                for hash_algorithm_str in signature_candidates:
+                    hash_algorithm = self.hash_algs[hash_algorithm_str]
+                    generated_hash = self.expressHMAC(payload, secret, hash_algorithm)
+                    if generated_hash == signature:
+                        return {
+                            "hash algorithm": hash_algorithm.__name__.split("openssl_")[1],
+                        }
         return False
 
     def check_secret(self, express_signed_cookie):

diff --git a/tests/express_signedcookies_test.py b/tests/express_signedcookies_test.py
@@ -29,6 +29,13 @@ def test_express_es():
         assert found_key["secret"] == test[0]
 
 
+def test_express_es_bad():
+    x = ExpressSignedCookies_ES()
+    for test in es_tests:
+        found_key = x.check_secret("s%3A%2F%2Fsomeorg.org%2Flocations%2Fnorth")
+        assert not found_key
+
+
 def test_express_cs():
     x = ExpressSignedCookies_CS()
     for test in cs_tests:

diff --git a/tests/module_consistency_test.py b/tests/module_consistency_test.py
@@ -7,4 +7,4 @@ def test_module_descriptions():
         assert m.get_description()["product"] != "Undefined"
         assert m.get_description()["secret"] != "Undefined"
         assert m.get_description()["severity"] != "Undefined"
-        assert m.get_description()["severity"] in ["INFORMATIONAL", "LOW", "MEDIUM", "HIGH", "CRITICAL"]
+        assert m.get_description()["severity"] in ["INFO", "LOW", "MEDIUM", "HIGH", "CRITICAL"]