From 8d9a9be63391218b8f2d94bb81b50956038dc39e Mon Sep 17 00:00:00 2001 From: Eric Brown Date: Tue, 6 Feb 2024 16:05:19 -0800 Subject: [PATCH] Make the artifact argument of a Result to be lazy set (#271) In order to simplify the rules, this change reduces the need to pass the file artifact when initializing the Result. This amounts to less code in each Rule. The artifact still gets set, but later in the parser. As a result, the snippet and artifact URI are also set later. Signed-off-by: Eric Brown --- precli/core/result.py | 40 ++++++++++++++----- precli/parsers/__init__.py | 4 +- precli/rules/go/stdlib/crypto_weak_cipher.py | 1 - precli/rules/go/stdlib/crypto_weak_hash.py | 2 - precli/rules/go/stdlib/crypto_weak_key.py | 2 - precli/rules/python/stdlib/crypt_weak_hash.py | 4 +- .../rules/python/stdlib/ftplib_cleartext.py | 3 -- .../rules/python/stdlib/hashlib_weak_hash.py | 3 -- .../rules/python/stdlib/hmac_timing_attack.py | 1 - precli/rules/python/stdlib/hmac_weak_hash.py | 2 - .../rules/python/stdlib/imaplib_cleartext.py | 3 +- precli/rules/python/stdlib/json_load.py | 1 - .../stdlib/logging_insecure_listen_config.py | 1 - precli/rules/python/stdlib/marshal_load.py | 1 - .../rules/python/stdlib/nntplib_cleartext.py | 1 - precli/rules/python/stdlib/pickle_load.py | 1 - .../rules/python/stdlib/poplib_cleartext.py | 1 - precli/rules/python/stdlib/shelve_open.py | 1 - .../rules/python/stdlib/smtplib_cleartext.py | 1 - .../python/stdlib/ssl_context_weak_key.py | 1 - .../stdlib/ssl_create_unverified_context.py | 1 - .../python/stdlib/ssl_insecure_tls_version.py | 5 +-- .../python/stdlib/telnetlib_cleartext.py | 1 - .../stdlib/tempfile_mktemp_race_condition.py | 3 +- 24 files changed, 37 insertions(+), 47 deletions(-) diff --git a/precli/core/result.py b/precli/core/result.py index 37867177..7a70e13c 100644 --- a/precli/core/result.py +++ b/precli/core/result.py @@ -13,10 +13,10 @@ class Result: def __init__( self, rule_id: str, + location: Location, artifact: Artifact = None, kind: Kind = Kind.FAIL, level: Level = None, - location: Location = None, message: str = None, fixes: list[Fix] = None, suppression: Suppression = None, @@ -38,25 +38,34 @@ def __init__( self._message = Rule.get_by_id(self._rule_id).message self._fixes = fixes if fixes is not None else [] self._suppression = suppression - if snippet is not None: self._snippet = snippet else: + self._init_snippet() + self._init_uri() + + def _init_snippet(self): + if self._artifact is not None and self._location is not None: linecache = LineCache( - artifact.file_name, - artifact.contents.decode(), + self._artifact.file_name, + self._artifact.contents.decode(), ) self._snippet = "" - for i in range(location.start_line - 1, location.end_line + 2): + for i in range( + self._location.start_line - 1, self._location.end_line + 2 + ): self._snippet += linecache.getline(i) + def _init_uri(self): # Append location info to the URI - if artifact.uri is not None: - if location.start_line != location.end_line: - lines = f"L{location.start_line}-L{location.end_line}" + if self._artifact is not None and self._artifact.uri is not None: + if self._location.start_line != self._location.end_line: + lines = ( + f"L{self._location.start_line}-L{self._location.end_line}" + ) else: - lines = f"L{location.start_line}" - artifact.uri = f"{artifact.uri}#{lines}" + lines = f"L{self._location.start_line}" + self._artifact.uri = f"{self._artifact.uri}#{lines}" @property def rule_id(self) -> str: @@ -81,6 +90,17 @@ def artifact(self) -> Artifact: """ return self._artifact + @artifact.setter + def artifact(self, artifact): + """ + Set the file artifact. + + :param Artifact artifact: file artifact + """ + self._artifact = artifact + self._init_snippet() + self._init_uri() + @property def location(self) -> Location: """ diff --git a/precli/parsers/__init__.py b/precli/parsers/__init__.py index 623a139f..5415fa9e 100644 --- a/precli/parsers/__init__.py +++ b/precli/parsers/__init__.py @@ -77,7 +77,7 @@ def parse(self, artifact: Artifact) -> list[Result]: :rtype: list """ self.results = [] - self.context = {"artifact": artifact} + self.context = {} if artifact.contents is None: with open(artifact.file_name, "rb") as fdata: artifact.contents = fdata.read() @@ -85,6 +85,8 @@ def parse(self, artifact: Artifact) -> list[Result]: self.visit([tree.root_node]) for result in self.results: + result.artifact = artifact + suppression = self.suppressions.get(result.location.start_line) if suppression and result.rule_id in suppression.rules: result.suppression = suppression diff --git a/precli/rules/go/stdlib/crypto_weak_cipher.py b/precli/rules/go/stdlib/crypto_weak_cipher.py index 1ed69d84..6ec251e8 100644 --- a/precli/rules/go/stdlib/crypto_weak_cipher.py +++ b/precli/rules/go/stdlib/crypto_weak_cipher.py @@ -158,7 +158,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: ) return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), level=Level.ERROR, message=self.message.format(call.name), diff --git a/precli/rules/go/stdlib/crypto_weak_hash.py b/precli/rules/go/stdlib/crypto_weak_hash.py index 8e83e5f9..7e1a00c4 100644 --- a/precli/rules/go/stdlib/crypto_weak_hash.py +++ b/precli/rules/go/stdlib/crypto_weak_hash.py @@ -106,7 +106,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: ) return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), level=Level.ERROR, message=self.message.format(call.name_qualified), @@ -124,7 +123,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: ) return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), level=Level.ERROR, message=self.message.format(call.name_qualified), diff --git a/precli/rules/go/stdlib/crypto_weak_key.py b/precli/rules/go/stdlib/crypto_weak_key.py index 9f6e3d0d..7b07a651 100644 --- a/precli/rules/go/stdlib/crypto_weak_key.py +++ b/precli/rules/go/stdlib/crypto_weak_key.py @@ -125,7 +125,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=argument.identifier_node), level=Level.ERROR, message=self.message.format("DSA", 2048), @@ -145,7 +144,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=argument.node), level=Level.ERROR if bits <= 1024 else Level.WARNING, message=self.message.format("RSA", 2048), diff --git a/precli/rules/python/stdlib/crypt_weak_hash.py b/precli/rules/python/stdlib/crypt_weak_hash.py index 197d9358..7213b609 100644 --- a/precli/rules/python/stdlib/crypt_weak_hash.py +++ b/precli/rules/python/stdlib/crypt_weak_hash.py @@ -1,4 +1,4 @@ -# Copyright 2023 Secure Saurce LLC +# Copyright 2024 Secure Saurce LLC r""" ======================================= Reversible One Way Hash in Crypt Module @@ -134,7 +134,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: if isinstance(name, str) and name in WEAK_CRYPT_HASHES: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), message=self.message.format(name), ) @@ -144,7 +143,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: if isinstance(name, str) and name in WEAK_CRYPT_HASHES: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), message=self.message.format(name), ) diff --git a/precli/rules/python/stdlib/ftplib_cleartext.py b/precli/rules/python/stdlib/ftplib_cleartext.py index cd73f0b2..2a1074f4 100644 --- a/precli/rules/python/stdlib/ftplib_cleartext.py +++ b/precli/rules/python/stdlib/ftplib_cleartext.py @@ -154,7 +154,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: if call.get_argument(position=2, name="passwd").value is not None: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), level=Level.ERROR, message=f"The '{call.name_qualified}' module will " @@ -164,7 +163,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: else: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), message=self.message.format(call.name_qualified), fixes=fixes, @@ -176,7 +174,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: if call.get_argument(position=1, name="passwd").value is not None: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.identifier_node), level=Level.ERROR, message=f"The '{call.name_qualified}' function will " diff --git a/precli/rules/python/stdlib/hashlib_weak_hash.py b/precli/rules/python/stdlib/hashlib_weak_hash.py index 753dd63c..e7a2ab87 100644 --- a/precli/rules/python/stdlib/hashlib_weak_hash.py +++ b/precli/rules/python/stdlib/hashlib_weak_hash.py @@ -137,7 +137,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: if used_for_security is True: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), level=Level.ERROR, message=self.message.format(call.name_qualified), @@ -157,7 +156,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: if isinstance(hash_name, str) and hash_name.lower() in WEAK_HASHES: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), level=Level.ERROR, message=self.message.format(hash_name), @@ -176,7 +174,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: if used_for_security is True: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), level=Level.ERROR, message=self.message.format(name), diff --git a/precli/rules/python/stdlib/hmac_timing_attack.py b/precli/rules/python/stdlib/hmac_timing_attack.py index 2571fff3..72bb053d 100644 --- a/precli/rules/python/stdlib/hmac_timing_attack.py +++ b/precli/rules/python/stdlib/hmac_timing_attack.py @@ -134,7 +134,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=comparison.operator_node), level=Level.ERROR, fixes=fixes, diff --git a/precli/rules/python/stdlib/hmac_weak_hash.py b/precli/rules/python/stdlib/hmac_weak_hash.py index d0313d87..75629f72 100644 --- a/precli/rules/python/stdlib/hmac_weak_hash.py +++ b/precli/rules/python/stdlib/hmac_weak_hash.py @@ -123,7 +123,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: ) or digestmod in HASHLIB_WEAK_HASHES: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=argument.node), level=Level.ERROR, message=self.message.format(digestmod), @@ -140,7 +139,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: ) or digest in HASHLIB_WEAK_HASHES: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=argument.node), level=Level.ERROR, message=self.message.format(digest), diff --git a/precli/rules/python/stdlib/imaplib_cleartext.py b/precli/rules/python/stdlib/imaplib_cleartext.py index eb9e5da8..272de603 100644 --- a/precli/rules/python/stdlib/imaplib_cleartext.py +++ b/precli/rules/python/stdlib/imaplib_cleartext.py @@ -1,4 +1,4 @@ -# Copyright 2023 Secure Saurce LLC +# Copyright 2024 Secure Saurce LLC r""" ===================================================================== Cleartext Transmission of Sensitive Information in the Imaplib Module @@ -116,7 +116,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.identifier_node), level=Level.ERROR, message=f"The '{call.name_qualified}' function will " diff --git a/precli/rules/python/stdlib/json_load.py b/precli/rules/python/stdlib/json_load.py index f01ee462..28f51099 100644 --- a/precli/rules/python/stdlib/json_load.py +++ b/precli/rules/python/stdlib/json_load.py @@ -109,7 +109,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: """ return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), message=self.message.format(call.name_qualified), ) diff --git a/precli/rules/python/stdlib/logging_insecure_listen_config.py b/precli/rules/python/stdlib/logging_insecure_listen_config.py index 4112f7f9..9d359d2c 100644 --- a/precli/rules/python/stdlib/logging_insecure_listen_config.py +++ b/precli/rules/python/stdlib/logging_insecure_listen_config.py @@ -83,7 +83,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: if call.get_argument(position=1, name="verify").value is None: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), message=self.message.format(call.name_qualified), ) diff --git a/precli/rules/python/stdlib/marshal_load.py b/precli/rules/python/stdlib/marshal_load.py index c14cb651..f7674336 100644 --- a/precli/rules/python/stdlib/marshal_load.py +++ b/precli/rules/python/stdlib/marshal_load.py @@ -78,7 +78,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: """ return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), message=self.message.format(call.name_qualified), ) diff --git a/precli/rules/python/stdlib/nntplib_cleartext.py b/precli/rules/python/stdlib/nntplib_cleartext.py index b0df7993..0630b331 100644 --- a/precli/rules/python/stdlib/nntplib_cleartext.py +++ b/precli/rules/python/stdlib/nntplib_cleartext.py @@ -97,7 +97,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.identifier_node), level=Level.ERROR, message=f"The '{call.name_qualified}' function will " diff --git a/precli/rules/python/stdlib/pickle_load.py b/precli/rules/python/stdlib/pickle_load.py index 99a23f9b..76ebfa1e 100644 --- a/precli/rules/python/stdlib/pickle_load.py +++ b/precli/rules/python/stdlib/pickle_load.py @@ -90,7 +90,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: ]: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), message=self.message.format(call.name_qualified), ) diff --git a/precli/rules/python/stdlib/poplib_cleartext.py b/precli/rules/python/stdlib/poplib_cleartext.py index 8471aeb8..b50cba63 100644 --- a/precli/rules/python/stdlib/poplib_cleartext.py +++ b/precli/rules/python/stdlib/poplib_cleartext.py @@ -113,7 +113,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.identifier_node), level=Level.ERROR, message=f"The '{call.name_qualified}' function will " diff --git a/precli/rules/python/stdlib/shelve_open.py b/precli/rules/python/stdlib/shelve_open.py index 10d1ac0d..75ef98db 100644 --- a/precli/rules/python/stdlib/shelve_open.py +++ b/precli/rules/python/stdlib/shelve_open.py @@ -72,7 +72,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: if call.name_qualified in ["shelve.open", "shelve.DbfilenameShelf"]: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), message=self.message.format(call.name_qualified), ) diff --git a/precli/rules/python/stdlib/smtplib_cleartext.py b/precli/rules/python/stdlib/smtplib_cleartext.py index 910cc6cd..161673db 100644 --- a/precli/rules/python/stdlib/smtplib_cleartext.py +++ b/precli/rules/python/stdlib/smtplib_cleartext.py @@ -144,7 +144,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.identifier_node), level=Level.ERROR, message=f"The '{call.name_qualified}' function will " diff --git a/precli/rules/python/stdlib/ssl_context_weak_key.py b/precli/rules/python/stdlib/ssl_context_weak_key.py index 1e525471..fd463f1a 100644 --- a/precli/rules/python/stdlib/ssl_context_weak_key.py +++ b/precli/rules/python/stdlib/ssl_context_weak_key.py @@ -110,7 +110,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=arg.node), level=Level.ERROR if key_size < 160 else Level.WARNING, message=self.message.format("EC", 224), diff --git a/precli/rules/python/stdlib/ssl_create_unverified_context.py b/precli/rules/python/stdlib/ssl_create_unverified_context.py index 9c16d22a..74d5df88 100644 --- a/precli/rules/python/stdlib/ssl_create_unverified_context.py +++ b/precli/rules/python/stdlib/ssl_create_unverified_context.py @@ -116,7 +116,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: ) return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), message=self.message.format(call.name_qualified), fixes=fixes, diff --git a/precli/rules/python/stdlib/ssl_insecure_tls_version.py b/precli/rules/python/stdlib/ssl_insecure_tls_version.py index 56752aeb..2c58ceac 100644 --- a/precli/rules/python/stdlib/ssl_insecure_tls_version.py +++ b/precli/rules/python/stdlib/ssl_insecure_tls_version.py @@ -1,4 +1,4 @@ -# Copyright 2023 Secure Saurce LLC +# Copyright 2024 Secure Saurce LLC r""" ======================================================= Inadequate Encryption Strength Using Weak SSL Protocols @@ -124,7 +124,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: ) return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=argument.identifier_node), level=Level.ERROR, message=self.message.format(version), @@ -165,7 +164,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: ) return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=argument.identifier_node), level=Level.ERROR, message=self.message.format(version), @@ -193,7 +191,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: ) return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=argument.identifier_node), level=Level.ERROR, message=self.message.format(protocol), diff --git a/precli/rules/python/stdlib/telnetlib_cleartext.py b/precli/rules/python/stdlib/telnetlib_cleartext.py index 1739a145..d0abf911 100644 --- a/precli/rules/python/stdlib/telnetlib_cleartext.py +++ b/precli/rules/python/stdlib/telnetlib_cleartext.py @@ -138,7 +138,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: if call.name_qualified in ["telnetlib.Telnet"]: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), level=Level.ERROR, message=self.message.format(call.name_qualified), diff --git a/precli/rules/python/stdlib/tempfile_mktemp_race_condition.py b/precli/rules/python/stdlib/tempfile_mktemp_race_condition.py index ec82699f..f8e5c433 100644 --- a/precli/rules/python/stdlib/tempfile_mktemp_race_condition.py +++ b/precli/rules/python/stdlib/tempfile_mktemp_race_condition.py @@ -1,4 +1,4 @@ -# Copyright 2023 Secure Saurce LLC +# Copyright 2024 Secure Saurce LLC r""" ============================================== Insecure Temporary File in the Tempfile Module @@ -178,7 +178,6 @@ def analyze(self, context: dict, **kwargs: dict) -> Result: return Result( rule_id=self.id, - artifact=context["artifact"], location=Location(node=call.function_node), message=self.message.format(file_arg.value), fixes=fixes,