42 code example where tested with various SAST tools detecting flaws: (#653)

* 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 <[email protected]>

* 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 <[email protected]>

---------

Signed-off-by: Helge Wehder <[email protected]>

Author: myteron

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(),

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(),

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 """

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)

docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/compliant02.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)

docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/example01.py

@@ -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

docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-665/README.md

@@ -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)
 
 

docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-665/example01.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