Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
8 changes: 4 additions & 4 deletions docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 """
Expand Down Expand Up @@ -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 """
Expand Down
61 changes: 28 additions & 33 deletions docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/compliant01.py
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -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)
83 changes: 0 additions & 83 deletions docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/compliant02.py

This file was deleted.

88 changes: 88 additions & 0 deletions docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-502/example01.py
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-665/example01.py

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the proposal to move this code into documentation.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading