From d74ee967838ddae34764fdb5ab21c53439f48dfc Mon Sep 17 00:00:00 2001 From: Helge Wehder Date: Fri, 11 Oct 2024 11:18:42 +0100 Subject: [PATCH 1/2] 42 code example where tested with various SAST tools detecting flaws: - PyLint 6 - Bandit 3 - Pyre 1 - flake8 0 - Pysa 0 Some type hint related flaws are corrected as part of this PR fixed wrong type hints in: CWE-664/CWE-134/compliant01.py CWE-664/CWE-843/compliant01.py CWE-682/CWE-1335/01/compliant02.py CWE-693/CWE-184/compliant01.py CWE-693/CWE-184/noncompliant01.py renamed CWE-664/CWE-502/compliant01.py to example as it does not represent compliance. Signed-off-by: Helge Wehder --- .../CWE-664/CWE-134/README.md | 4 +- .../CWE-664/CWE-134/compliant01.py | 2 +- .../CWE-664/CWE-502/README.md | 8 +- .../CWE-664/CWE-502/compliant01.py | 61 ++++++------- .../CWE-664/CWE-502/compliant02.py | 83 ----------------- .../CWE-664/CWE-502/example01.py | 88 +++++++++++++++++++ .../CWE-664/CWE-843/compliant01.py | 2 +- .../CWE-664/CWE-843/noncompliant01.py | 2 +- .../CWE-693/CWE-184/compliant01.py | 11 +-- .../CWE-693/CWE-184/noncompliant01.py | 9 +- 10 files changed, 137 insertions(+), 133 deletions(-) delete mode 100644 docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/compliant02.py create mode 100644 docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/example01.py diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-134/README.md b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-134/README.md index e21c366c..203c0474 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-134/README.md +++ b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-134/README.md @@ -64,6 +64,8 @@ The `compliant01.py` solution uses the string template module and avoids mixing *[compliant01.py](compliant01.py):* ```python +# SPDX-FileCopyrightText: OpenSSF project contributors +# SPDX-License-Identifier: MIT """ Compliant Code Example """ import sys from string import Template @@ -85,7 +87,7 @@ class MicroService: return self.instance_name -def front_end(customer) -> str: +def front_end(customer): """Display service instance""" mc = MicroService("big time microservice") print(MESSAGE.substitute(instance_name=mc.get_instance_name(), diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-134/compliant01.py b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-134/compliant01.py index 01321048..836319e3 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-134/compliant01.py +++ b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-134/compliant01.py @@ -21,7 +21,7 @@ def get_instance_name(self) -> str: return self.instance_name -def front_end(customer) -> str: +def front_end(customer): """Display service instance""" mc = MicroService("big time microservice") print(MESSAGE.substitute(instance_name=mc.get_instance_name(), diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/README.md b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/README.md index 8e8cdffd..479c5791 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/README.md +++ b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/README.md @@ -97,11 +97,11 @@ message = p3.uncan(PAYLOAD) The deserializating `Preserver.uncan()` method has no solution to verify the content prior to unpickling it and runs the PAYLOAD even before turning it into an object. On Windows you have `calc.exe` and on Unix a bunch of commands such as `uname -a and ls -la /etc/shadow`. > [!CAUTION] -> The `compliant01.py` code only demonstrates integrity protection with hmac. +> The `example01.py` code only demonstrates integrity protection with hmac. > The pickled object is not encrypted and key-handling is inappropriate! > Consider using proper key management with `x509` and encryption [[pyca/cryptography 2023]](https://cryptography.io/en/latest/). -*[compliant01.py](compliant01.py):* +*[example01.py](example01.py):* ```py """ Compliant Code Example """ @@ -198,11 +198,11 @@ The integrity verification in `compliant01.py` throws an exception `ValueError: Text-based formats, such as `JSON` and `YAML`, should always be preferred. They have a lower set of capabilities and reduce the attack surface [python.org comparison-with-json 2023] when compared to `pickle`. -The `compliant02.py` code only allows serializing and deserialization of object data and not object methods as in `noncompliant01.py` or `compliant01.py`. +The `compliant01.py` code only allows serializing and deserialization of object data and not object methods as in `noncompliant01.py` or `compliant01.py`. Consider converting binary data into text using `Base64` encoding for performance and size irrelevant operations. -*[compliant02.py](compliant02.py):* +*[compliant01.py](compliant01.py):* ```py """ Compliant Code Example """ diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/compliant01.py b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/compliant01.py index 2de6898e..b4a82682 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/compliant01.py +++ b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/compliant01.py @@ -1,74 +1,69 @@ # SPDX-FileCopyrightText: OpenSSF project contributors # SPDX-License-Identifier: MIT """ Compliant Code Example """ -import hashlib -import hmac import platform -import pickle -import secrets +import json class Message(object): """Sample Message Object""" - sender_id = 42 - text = "Some text" + sender_id = int() + text = str() + + def __init__(self): + self.sender_id = 42 + self.text = "Some text" def printout(self): - """prints content to stdout to demonstrate active content""" - print(f"Message:sender_id={self.sender_id} text={self.text}") + print(f"sender_id: {self.sender_id}\ntext: {self.text}") class Preserver(object): """Demonstrating deserialisation""" - def __init__(self, _key): - self._key = _key - def can(self, _message: Message) -> tuple: + def can(self, _message: Message) -> str: """Serializes a Message object. Parameters: _message (Message): Message object Returns: - _digest (String): HMAC digest string - _jar (bytes): pickled jar as string + _jar (bytes): jar as string """ - _jar = pickle.dumps(_message) - _digest = hmac.new(self._key, _jar, hashlib.sha256).hexdigest() - return _digest, _jar + return json.dumps(vars(_message)) - def uncan(self, _expected_digest, _jar) -> Message: + def uncan(self, _jar) -> Message: """Verifies and de-serializes a Message object. Parameters: - _expected_digest (String): Message HMAC digest - _jar (bytes): Pickled jar + _jar (String): Pickled jar Returns: (Message): Message object """ - _digest = hmac.new(self._key, _jar, hashlib.sha256).hexdigest() - if _expected_digest != _digest: - raise ValueError("Integrity of jar compromised") - return pickle.loads(_jar) + j = json.loads(_jar) + _message = Message() + _message.sender_id = int(j["sender_id"]) + _message.text = str(j["text"]) + return _message # serialization of a normal package -key = secrets.token_bytes() -print(f"key={key}") -p1 = Preserver(key) +p1 = Preserver() message = Message() -message.printout() -digest, jar = p1.can(message) +jar = p1.can(message) +print(jar) +print(type(json.loads(jar))) # sending or storing would happen here -p2 = Preserver(key) +p2 = Preserver() message = None -message = p2.uncan(digest, jar) +message = p2.uncan(jar) message.printout() +print(message.sender_id) ##################### # exploiting above code example ##################### print("-" * 10) print("Attacker trying to read the message") -message = pickle.loads(jar) +print(jar) message.printout() print("-" * 10) @@ -83,6 +78,6 @@ def uncan(self, _expected_digest, _jar) -> Message: (S'whoami;uptime;uname -a;ls -la /etc/shadow' tR.""" print("Attacker trying to inject PAYLOAD") -p3 = Preserver(b"dont know") +p3 = Preserver() message = None -message = p3.uncan(digest, PAYLOAD) +message = p3.uncan(PAYLOAD) diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/compliant02.py b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/compliant02.py deleted file mode 100644 index b4a82682..00000000 --- a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/compliant02.py +++ /dev/null @@ -1,83 +0,0 @@ -# SPDX-FileCopyrightText: OpenSSF project contributors -# SPDX-License-Identifier: MIT -""" Compliant Code Example """ -import platform -import json - - -class Message(object): - """Sample Message Object""" - sender_id = int() - text = str() - - def __init__(self): - self.sender_id = 42 - self.text = "Some text" - - def printout(self): - print(f"sender_id: {self.sender_id}\ntext: {self.text}") - - -class Preserver(object): - """Demonstrating deserialisation""" - - def can(self, _message: Message) -> str: - """Serializes a Message object. - Parameters: - _message (Message): Message object - Returns: - _jar (bytes): jar as string - """ - return json.dumps(vars(_message)) - - def uncan(self, _jar) -> Message: - """Verifies and de-serializes a Message object. - Parameters: - _jar (String): Pickled jar - Returns: - (Message): Message object - """ - j = json.loads(_jar) - _message = Message() - _message.sender_id = int(j["sender_id"]) - _message.text = str(j["text"]) - return _message - - -# serialization of a normal package -p1 = Preserver() -message = Message() -jar = p1.can(message) -print(jar) -print(type(json.loads(jar))) - -# sending or storing would happen here -p2 = Preserver() -message = None -message = p2.uncan(jar) -message.printout() -print(message.sender_id) - -##################### -# exploiting above code example -##################### -print("-" * 10) -print("Attacker trying to read the message") -print(jar) -message.printout() - -print("-" * 10) -if platform.system() == "Windows": - PAYLOAD = b"""cos -system -(S'calc.exe' -tR.""" -else: - PAYLOAD = b"""cos -system -(S'whoami;uptime;uname -a;ls -la /etc/shadow' -tR.""" -print("Attacker trying to inject PAYLOAD") -p3 = Preserver() -message = None -message = p3.uncan(PAYLOAD) diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/example01.py b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/example01.py new file mode 100644 index 00000000..2de6898e --- /dev/null +++ b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/example01.py @@ -0,0 +1,88 @@ +# SPDX-FileCopyrightText: OpenSSF project contributors +# SPDX-License-Identifier: MIT +""" Compliant Code Example """ +import hashlib +import hmac +import platform +import pickle +import secrets + + +class Message(object): + """Sample Message Object""" + sender_id = 42 + text = "Some text" + + def printout(self): + """prints content to stdout to demonstrate active content""" + print(f"Message:sender_id={self.sender_id} text={self.text}") + + +class Preserver(object): + """Demonstrating deserialisation""" + def __init__(self, _key): + self._key = _key + + def can(self, _message: Message) -> tuple: + """Serializes a Message object. + Parameters: + _message (Message): Message object + Returns: + _digest (String): HMAC digest string + _jar (bytes): pickled jar as string + """ + _jar = pickle.dumps(_message) + _digest = hmac.new(self._key, _jar, hashlib.sha256).hexdigest() + return _digest, _jar + + def uncan(self, _expected_digest, _jar) -> Message: + """Verifies and de-serializes a Message object. + Parameters: + _expected_digest (String): Message HMAC digest + _jar (bytes): Pickled jar + Returns: + (Message): Message object + """ + _digest = hmac.new(self._key, _jar, hashlib.sha256).hexdigest() + if _expected_digest != _digest: + raise ValueError("Integrity of jar compromised") + return pickle.loads(_jar) + + +# serialization of a normal package +key = secrets.token_bytes() +print(f"key={key}") +p1 = Preserver(key) +message = Message() +message.printout() +digest, jar = p1.can(message) + +# sending or storing would happen here +p2 = Preserver(key) +message = None +message = p2.uncan(digest, jar) +message.printout() + +##################### +# exploiting above code example +##################### +print("-" * 10) +print("Attacker trying to read the message") +message = pickle.loads(jar) +message.printout() + +print("-" * 10) +if platform.system() == "Windows": + PAYLOAD = b"""cos +system +(S'calc.exe' +tR.""" +else: + PAYLOAD = b"""cos +system +(S'whoami;uptime;uname -a;ls -la /etc/shadow' +tR.""" +print("Attacker trying to inject PAYLOAD") +p3 = Preserver(b"dont know") +message = None +message = p3.uncan(digest, PAYLOAD) diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-843/compliant01.py b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-843/compliant01.py index 34072f0f..298535c9 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-843/compliant01.py +++ b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-843/compliant01.py @@ -3,7 +3,7 @@ """ Compliant Code Example """ -def shopping_bag(price: int, qty: int) -> int: +def shopping_bag(price: int, qty: str) -> int: return int(price) * int(qty) diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-843/noncompliant01.py b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-843/noncompliant01.py index 5b8986c9..e0c1a52d 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-843/noncompliant01.py +++ b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-843/noncompliant01.py @@ -3,7 +3,7 @@ """ Non-compliant Code Example """ -def shopping_bag(price: int, qty: int) -> int: +def shopping_bag(price: int, qty: str) -> int: return price * qty diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-693/CWE-184/compliant01.py b/docs/Secure-Coding-Guide-for-Python/CWE-693/CWE-184/compliant01.py index 7f6bd79b..7690a9fa 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-693/CWE-184/compliant01.py +++ b/docs/Secure-Coding-Guide-for-Python/CWE-693/CWE-184/compliant01.py @@ -5,17 +5,17 @@ import unicodedata import sys -sys.stdout.reconfigure(encoding='UTF-8') +sys.stdout.reconfigure(encoding="UTF-8") class TagFilter: """Input validation for human language""" def filter_string(self, input_string: str) -> str: - """ Normalize and validate untrusted string + """Normalize and validate untrusted string - Parameters: - input_string(string): String to validate + Parameters: + input_string(string): String to validate """ # normalize _str = unicodedata.normalize("NFKC", input_string) @@ -24,11 +24,12 @@ def filter_string(self, input_string: str) -> str: _filtered_str = "".join(re.findall(r"[/\w<>\s-]+", _str)) if len(_str) - len(_filtered_str) != 0: raise ValueError("Invalid input string") - + # validate, only allow harmless tags for tag in re.findall("<[^>]*>", _str): if tag not in ["", "

", "

"]: raise ValueError("Invalid input tag") + return _str ##################### diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-693/CWE-184/noncompliant01.py b/docs/Secure-Coding-Guide-for-Python/CWE-693/CWE-184/noncompliant01.py index c025b96c..259408ad 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-693/CWE-184/noncompliant01.py +++ b/docs/Secure-Coding-Guide-for-Python/CWE-693/CWE-184/noncompliant01.py @@ -5,17 +5,17 @@ import unicodedata import sys -sys.stdout.reconfigure(encoding='UTF-8') +sys.stdout.reconfigure(encoding="UTF-8") class TagFilter: """Input validation for human language""" def filter_string(self, input_string: str) -> str: - """ Normalize and validate untrusted string + """Normalize and validate untrusted string - Parameters: - input_string(string): String to validate + Parameters: + input_string(string): String to validate """ # normalize _str = unicodedata.normalize("NFKC", input_string) @@ -30,6 +30,7 @@ def filter_string(self, input_string: str) -> str: _filtered_str = "".join(re.findall(r"[/\w<>\s-]+", _str)) if len(_str) - len(_filtered_str) != 0: raise ValueError("Invalid input string") + return _filtered_str ##################### From 7dd078d3462eddcdf5edb3a5408446a3b5054a28 Mon Sep 17 00:00:00 2001 From: Helge Wehder Date: Wed, 16 Oct 2024 14:48:03 +0100 Subject: [PATCH 2/2] CWE-664/CWE-1335/compliant02.py corrected type hints CWE-664/665/ removed example01.py as its just a code snippet Signed-off-by: Helge Wehder --- .../CWE-664/CWE-665/README.md | 3 +-- .../CWE-664/CWE-665/example01.py | 10 ---------- .../CWE-682/CWE-1335/01/README.md | 8 ++++---- .../CWE-682/CWE-1335/01/compliant02.py | 7 +++---- 4 files changed, 8 insertions(+), 20 deletions(-) delete mode 100644 docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-665/example01.py diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-665/README.md b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-665/README.md index 2c14cd83..98fcde07 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-665/README.md +++ b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-665/README.md @@ -118,6 +118,7 @@ ThreadPoolExecutor-0_1: Working concurrently as User.ADMIN The two worker threads have been initialized only once using the initializer method. `ThreadPoolExecutor-0_1` has completed the `work_as_admin()` task and has been reused to complete one of the `work_as_guest()`. Because the local values have been changed by the first task, the changed values persisted to the second task. Table listing a possible execution order: + |Task|Workder Thread|Executed Method|User| |:---|:---|:---|:---| |1|0|`work_as_admin()`|ADMIN| @@ -207,8 +208,6 @@ for future in futures: The increased thread pool size circumvents the problem for this specific example. However, the problem has not been resolved since expanding the number of submitted tasks will cause it to reoccur. For example, if we change the `fututes` list as follows: -*[example01.py](example01.py):* - ```py futures = [ sp.execute_task(sp.work_as_admin), # Thread 1, works as ADMIN diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-665/example01.py b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-665/example01.py deleted file mode 100644 index f4ae9fda..00000000 --- a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-665/example01.py +++ /dev/null @@ -1,10 +0,0 @@ -# SPDX-FileCopyrightText: OpenSSF project contributors -# SPDX-License-Identifier: MIT -futures = [ - sp.execute_task(sp.work_as_admin), # Thread 1, works as ADMIN - sp.execute_task(sp.work_as_guest), # Thread 2, should work as GUEST - sp.execute_task(sp.work_as_admin), # Thread 1, works as ADMIN - sp.execute_task(sp.work_as_guest), # Thread 3, should work as GUEST - sp.execute_task(sp.work_as_guest), # Thread 2, should work as GUEST - sp.execute_task(sp.work_as_guest), # Thread 3, should work as GUEST -] diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-682/CWE-1335/01/README.md b/docs/Secure-Coding-Guide-for-Python/CWE-682/CWE-1335/01/README.md index fd5aaedf..8cabf53f 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-682/CWE-1335/01/README.md +++ b/docs/Secure-Coding-Guide-for-Python/CWE-682/CWE-1335/01/README.md @@ -120,10 +120,10 @@ The right shift is replaced by division in `compliant02.py`. ```py """ Compliant Code Example """ -foo: int -foo = -50 -foo /= 4 -print(foo) +foo: int = -50 +bar: float = foo / 4 +print(bar) + ``` ## Automated Detection diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-682/CWE-1335/01/compliant02.py b/docs/Secure-Coding-Guide-for-Python/CWE-682/CWE-1335/01/compliant02.py index 2b78c8c2..e81e210d 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-682/CWE-1335/01/compliant02.py +++ b/docs/Secure-Coding-Guide-for-Python/CWE-682/CWE-1335/01/compliant02.py @@ -2,7 +2,6 @@ # SPDX-License-Identifier: MIT """ Compliant Code Example """ -foo: int -foo = -50 -foo /= 4 -print(foo) +foo: int = -50 +bar: float = foo / 4 +print(bar)