From b5aad2520e6d841eaa2f67e37c4431b2990eebfa Mon Sep 17 00:00:00 2001 From: emcdtho Date: Wed, 5 Jun 2024 14:13:31 +0100 Subject: [PATCH] Fixes #519: code examples use Jekyll Signed-off-by: emcdtho --- .../CWE-664/CWE-400/README.md | 77 +----- .../CWE-664/CWE-410/README.md | 243 +---------------- .../CWE-664/CWE-502/README.md | 248 +----------------- .../CWE-664/CWE-833/README.md | 198 +------------- .../CWE-682/CWE-1335/01/README.md | 53 +--- .../CWE-703/CWE-392/README.md | 171 +----------- 6 files changed, 51 insertions(+), 939 deletions(-) diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-400/README.md b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-400/README.md index d2ea79e0..a98faace 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-400/README.md +++ b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-400/README.md @@ -8,39 +8,8 @@ Tasks can be submitted to the ThreadPoolExecutor by calling `submit()`. Submitte [*noncompliant01.py:*](noncompliant01.py) -```py -""" Non-compliant Code Example """ -import time -from concurrent.futures import ThreadPoolExecutor - - -def take_time(x): - print(f"Started Task: {x}") - # Simulate work - for i in range(10): - time.sleep(1) - print(f"Completed Task: {x}") - - -def run_thread(_executor, var): - future = _executor.submit(take_time, var) - return future - - -def interrupt(future): - print(future.cancel()) - print(f"Interrupted: {future}") - - -##################### -# Exploiting above code example -##################### - - -with ThreadPoolExecutor() as executor: - task = run_thread(executor, "A") - interrupt(task) - +```python +{% include_relative noncompliant01.py %} ``` ## Compliant Solution @@ -49,46 +18,8 @@ Tasks submitted to the ThreadPoolExecutor can be interrupted by setting a thread [*compliant01.py:*](compliant01.py) -```py -""" Compliant Code Example """ -import time -from concurrent.futures import ThreadPoolExecutor -from threading import Event - - -def take_time(x, _event): - print(f"Started Task: {x}") - # Simulate work - for _ in range(10): - if _event.is_set(): - print(f"Interrupted Task: {x}") - # Save partial results - return - time.sleep(1) - print(f"Completed Task: {x}") - - -def run_thread(_executor, var): - e = Event() - future = _executor.submit(take_time, var, e) - return future, e - - -def interrupt(future, e): - """Cancel the task, just in case it is not yet running, and set the Event flag""" - future.cancel() - e.set() - - -##################### -# Exploiting above code example -##################### - - -with ThreadPoolExecutor() as executor: - task, event = run_thread(executor, "A") - interrupt(task, event) - +```python +{% include_relative compliant01.py %} ``` ## Related Guidelines diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-410/README.md b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-410/README.md index 0caa2551..c249f30c 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-410/README.md +++ b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-410/README.md @@ -14,55 +14,8 @@ The `noncompliant01.py` code example demonstrates the Thread-Per-Message design *[noncompliant01.py](noncompliant01.py):* -```py -""" Non-compliant Code Example """ -import logging -import threading -import time - -logging.basicConfig(level=logging.INFO) - - -def process_message(message: str, processed_messages: list): - """ Method simulating mediation layer i/o heavy work""" - logging.debug("process_message: started message %s working %is", message, int(message) / 10) - for _ in range(int(message)): - time.sleep(0.01) - logging.debug("process_message: completed message %s", message) - processed_messages.append(f"processed {message}") - - -class MessageAPI(object): - """Class simulating the front end facing API""" - - def add_messages(self, messages: list) -> list: - """ Receives a list of messages to work on """ - logging.info("add_messages: got %i messages to process", len(messages)) - processed_messages = [] - threads = [] - for message in messages: - threads.append( - threading.Thread(target=process_message, args=[message, processed_messages])) - threads[-1].start() - logging.debug("add_messages: submitted %i messages", len(messages)) - for thread in threads: - thread.join() - logging.info("add_messages: messages_done=%i", len(processed_messages)) - return processed_messages - - -##################### -# exploiting above code example -##################### -mapi = MessageAPI() -attacker_messages = [str(msg) for msg in range(1000)] -print("ATTACKER: start sending messages") -result_list = mapi.add_messages(attacker_messages) -print( - f"ATTACKER: done sending {len(attacker_messages)} messages, got {len(result_list)} messages " - f"back") -print(f"ATTACKER: result_list = {result_list}") - +```python +{% include_relative noncompliant01.py %} ``` The `noncompliant01.py` code creates a risk of having a single component exhaust all available hardware resources even when used by a single user for a single use-case. When running the code, the Unix `top` command can show a significant increase in CPU usage (%CPU exceeds 100% because multiple cores are being used): @@ -80,64 +33,8 @@ The attacker could still flood the server by creating multiple MessageAPI, each *[compliant01.py](compliant01.py)* -```py -""" Compliant Code Example """ -import logging -import time -from concurrent.futures import ThreadPoolExecutor -from concurrent.futures import wait - -logging.basicConfig(level=logging.INFO) - - -def process_message(message: str): - """ Method simulating mediation layer i/o heavy work""" - logging.debug("process_message: started message %s working %is", message, int(message) / 10) - for _ in range(int(message)): - time.sleep(0.01) - logging.debug("process_message: completed message %s", message) - return f"processed {message}" - - -class MessageAPI(object): - """Class simulating the front end facing API""" - # TODO: Prevent the attacker from creating multiple MessageAPI objects - - def __init__(self): - # TODO: set or handle timeout as it is provided by the mediation layer - self.timeout = 1 - self.executor = ThreadPoolExecutor() - - def add_messages(self, messages: list) -> list: - """ Receives a list of messages to work on """ - # TODO: limit on max messages from the mediation layer. - # TODO: input sanitation. - futures = [] - # with self.executor: - for message in messages: - futures.append(self.executor.submit(process_message, message)) - logging.debug("add_messages: submitted %i messages, waiting for %is to complete.", len(messages), self.timeout) - messages_done, messages_not_done = wait(futures, timeout=self.timeout) - for future in messages_not_done: - future.cancel() - - logging.info("add_messages: messages_done=%i messages_not_done=%i", len(messages_done), len(messages_not_done)) - process_messages = [] - for future in messages_done: - process_messages.append(future.result()) - return process_messages - - -##################### -# exploiting above code example -##################### -mapi = MessageAPI() -result_list = [] -attacker_messages = [str(msg) for msg in range(100)] -print("ATTACKER: start sending messages") -result_list = mapi.add_messages(attacker_messages) -print(f"ATTACKER: done sending {len(attacker_messages)} messages, got {len(result_list)} messages back") -print(f"ATTACKER: result_list = {result_list}") +```python +{% include_relative compliant01.py %} ``` Now, after the timeout is reached, `MessageAPI` drops unprocessed messages and returns partial results: @@ -155,63 +52,8 @@ The `executor.shutdown()` method has a `cancel_futures` parameter, which by def *[noncompliant02.py](noncompliant02.py):* -```py -""" Non-compliant Code Example """ -import logging -import time -from concurrent.futures import ThreadPoolExecutor -from concurrent.futures import wait - -logging.basicConfig(level=logging.INFO) - - -def process_message(message: str): - """ Method simulating mediation layer i/o heavy work""" - logging.debug("process_message: started message %s working %is", message, int(message) / 10) - for _ in range(int(message)): - time.sleep(0.01) - logging.debug("process_message: completed message %s", message) - return f"processed {message}" - - -class MessageAPI(object): - """Class simulating the front end facing API""" - - def __init__(self): - self.executor = ThreadPoolExecutor() - self.timeout = 1 - - def add_messages(self, messages: list) -> list: - """ Receives a list of messages to work on """ - futures = [] - for message in messages: - futures.append(self.executor.submit(process_message, message)) - - logging.debug("add_messages: submitted %i messages, waiting for %is to complete.", - len(messages), self.timeout) - messages_done, messages_not_done = wait(futures, timeout=self.timeout) - - logging.info("add_messages: messages_done=%i messages_not_done=%i", len(messages_done), - len(messages_not_done)) - - process_messages = [] - for future in messages_done: - process_messages.append(future.result()) - return process_messages - - -##################### -# exploiting above code example -##################### -mapi = MessageAPI() -result_list = [] -attacker_messages = [str(msg) for msg in range(1000)] -print("ATTACKER: start sending messages") -result_list = mapi.add_messages(attacker_messages) -print( - f"ATTACKER: done sending {len(attacker_messages)} messages, got {len(result_list)} messages " - f"back") -print(f"ATTACKER: result_list = {result_list}") +```python +{% include_relative noncompliant02.py %} ``` ## Compliant Solution (Thread Pool with grace period) @@ -220,77 +62,8 @@ The `compliant01.py` can be expanded by adding a grace period. Before dropping t *[compliant02.py](compliant02.py):* -```py -""" Compliant Code Example """ -import logging -import time -from concurrent.futures import ThreadPoolExecutor -from concurrent.futures import wait - -logging.basicConfig(level=logging.INFO) - - -def process_message(message: str): - """ Method simulating mediation layer i/o heavy work""" - logging.debug("process_message: started message %s working %is", message, int(message) / 10) - for _ in range(int(message)): - time.sleep(0.01) - logging.debug("process_message: completed message %s", message) - return f"processed {message}" - - -class MessageAPI(object): - """Class simulating the front end facing API""" - # TODO: Prevent the attacker from creating multiple MessageAPI objects - - def __init__(self): - # TODO: set or handle timeout as it is provided by the mediation layer - self.timeout = 1 - self.executor = ThreadPoolExecutor() - - def add_messages(self, messages: list) -> list: - """ Receives a list of messages to work on """ - # TODO: limit on max messages from the mediation layer. - # TODO: input sanitation. - futures = [] - # with self.executor: - for message in messages: - futures.append(self.executor.submit(process_message, message)) - logging.debug("add_messages: submitted %i messages, waiting for %is to complete.", - len(messages), self.timeout) - messages_done, messages_not_done = wait(futures, timeout=self.timeout) - logging.info("add_messages: messages_done=%i messages_not_done=%i", len(messages_done), - len(messages_not_done)) - if len(messages_not_done) > 0: - # TODO: be graceful, warn a trusted client - logging.warning("add_messages: %i messages taking longer than %is, %i more to process", - len(messages), self.timeout, len(messages_not_done)) - messages_done, messages_not_done = wait(futures, timeout=self.timeout) - logging.info("add_messages: messages_done=%i messages_not_done=%i", len(messages_done), - len(messages_not_done)) - for future in messages_not_done: - future.cancel() - - logging.info("add_messages: messages_done=%i messages_not_done=%i", len(messages_done), - len(messages_not_done)) - process_messages = [] - for future in messages_done: - process_messages.append(future.result()) - return process_messages - - -##################### -# exploiting above code example -##################### -mapi = MessageAPI() -result_list = [] -attacker_messages = [str(msg) for msg in range(100)] -print("ATTACKER: start sending messages") -result_list = mapi.add_messages(attacker_messages) -print( - f"ATTACKER: done sending {len(attacker_messages)} messages, got {len(result_list)} messages " - f"back") -print(f"ATTACKER: result_list = {result_list}") +```python +{% include_relative compliant02.py %} ``` ## Automated Detection 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..86d6ee50 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 @@ -19,79 +19,8 @@ The `noncompliant01.py` code demonstrates arbitrary code execution [Checkoway O *[noncompliant01.py](noncompliant01.py):* -```py -""" Non-Compliant Code Example """ -import platform -import pickle - - -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 can(self, _message: Message) -> bytes: - """Serializes a Message object. - Parameters: - _message (Message): Message object - Returns: - _jar (bytes): pickled jar as string - """ - return pickle.dumps(_message) - - def uncan(self, _jar) -> Message: - """De-serializes a Message object. - Parameters: - _jar (String): Pickled jar - Returns: - (Message): Message object - """ - return pickle.loads(_jar) - - -# serialization of a normal package -p1 = Preserver() -message = Message() -message.printout() -jar = p1.can(message) - -# sending or storing would happen here -p2 = Preserver() -message = None -message = p2.uncan(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() -message = None -message = p3.uncan(PAYLOAD) +```python +{% include_relative noncompliant01.py %} ``` 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`. @@ -103,93 +32,8 @@ The deserializating `Preserver.uncan()` method has no solution to verify the con *[compliant01.py](compliant01.py):* -```py -""" 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) +```python +{% include_relative compliant01.py %} ``` The integrity verification in `compliant01.py` throws an exception `ValueError: Integrity of jar compromised prior to deserializationunpickling to prevent the PAYLOAD executed.` @@ -204,88 +48,8 @@ Consider converting binary data into text using `Base64` encoding for performanc *[compliant02.py](compliant02.py):* -```py -""" 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) +```python +{% include_relative compliant02.py %} ``` The `compliant02.py` stops with the unpacking with a `json.decoder.JSONDecodeError`. diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-833/README.md b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-833/README.md index 54ab4a5c..508c4b42 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-833/README.md +++ b/docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-833/README.md @@ -8,48 +8,8 @@ The `noncompliant01.py` code example shows how a thread-starvation deadlock coul *[noncompliant01.py](noncompliant01.py):* -```py -""" Non-compliant Code Example """ - -from concurrent.futures import ThreadPoolExecutor -from typing import List - - -class ReportTableGenerator(object): - def __init__(self): - self.executor = ThreadPoolExecutor() - - def generate_string_table(self, inputs: List[str]) -> str: - futures = [] - aggregated = "|Data|Length|\n" - for i in inputs: - futures.append(self.executor.submit(self._create_table_row, i)) - for future in futures: - aggregated += future.result() - return aggregated - - def _create_table_row(self, row: str) -> str: - print(f"Creating a row out of: {row}") - future = self.executor.submit(self._reformat_string, row) - return f"|{future.result()}|{len(row)}|\n" - - def _reformat_string(self, row: str) -> str: - print(f"Reformatting {row}") - row_reformated = row.capitalize() - return row_reformated - - -##################### -# exploiting above code example -##################### -report_table_generator = ReportTableGenerator() -attacker_messages = [str(msg) for msg in range(1000)] -print("ATTACKER: start sending messages") -result = report_table_generator.generate_string_table(attacker_messages) -print( - f"ATTACKER: done sending {len(attacker_messages)} messages, got {len(result)} messages " - f"back") -print(f"ATTACKER: result = {result}") +```python +{% include_relative noncompliant01.py %} ``` The problem arises when the number of concurrently built rows exceeds the number of `_max_workers`. In this example, each worker thread could be occupied with the execution of the `_create_table_row()` method, while the tasks of `executing _reformat_string()` are waiting in the queue. Since the `_create_table_row()` method spawns a `_reformat_string()` thread and waits for its result, every worker will wait for another worker to finish their part of the row-building process. Because all workers are busy with the `_create_table_row()` task, no worker will be able to take the string reformatting out of the queue, causing a deadlock. @@ -60,47 +20,8 @@ The `compliant01.py` code example illustrates how interdependent tasks could be *[compliant01.py](compliant01.py):* -```py -""" Compliant Code Example """ - -from concurrent.futures import ThreadPoolExecutor -from typing import List - - -class ReportTableGenerator(object): - def __init__(self): - self.executor = ThreadPoolExecutor() - - def generate_string_table(self, inputs: List[str]) -> str: - futures = [] - aggregated = "|Data|Length|\n" - for i in inputs: - futures.append(self.executor.submit(self._create_table_row, i)) - for future in futures: - aggregated += future.result() - return aggregated - - def _create_table_row(self, row: str) -> str: - print(f"Creating a row out of: {row}") - return f"|{self._reformat_string(row)}|{len(row)}|\n" - - def _reformat_string(self, row: str) -> str: - print(f"Reformatting {row}") - row_reformated = row.capitalize() - return row_reformated - - -##################### -# exploiting above code example -##################### -report_table_generator = ReportTableGenerator() -attacker_messages = [str(msg) for msg in range(1000)] -print("ATTACKER: start sending messages") -result = report_table_generator.generate_string_table(attacker_messages) -print( - f"ATTACKER: done sending {len(attacker_messages)} messages, got {len(result)} messages " - f"back") -print(f"ATTACKER: result = {result}") +```python +{% include_relative compliant01.py %} ``` Now, the `_create_table_row()` method is executed in a separate thread but does not spawn any more threads, instead reformatting the string sequentially. Because each input argument can have its row built separately, no thread will need to wait for another to finish, which avoids thread-starvation deadlocks. @@ -112,50 +33,8 @@ The BankingService class allows to concurrently validate the cards of all of the *[noncompliant02.py](noncompliant02.py):* -```py -""" Non-compliant Code Example """ - -from concurrent.futures import ThreadPoolExecutor, wait -from threading import Lock -from typing import Callable - - -class BankingService(object): - def __init__(self, n: int): - self.executor = ThreadPoolExecutor() - self.number_of_times = n - self.count = 0 - self.lock = Lock() - - def for_each_client(self): - print("For each client") - self.invoke_method(self.for_each_account) - - def for_each_account(self): - print("For each account") - self.invoke_method(self.for_each_card) - - def for_each_card(self): - print("For each card") - self.invoke_method(self.check_card_validity) - - def check_card_validity(self): - with self.lock: - self.count += 1 - print(f"Number of checked cards: {self.count}") - - def invoke_method(self, method: Callable): - futures = [] - for _ in range(self.number_of_times): - futures.append(self.executor.submit(method)) - wait(futures) - - -##################### -# exploiting above code example -##################### -browser_manager = BankingService(5) -browser_manager.for_each_client() +```python +{% include_relative noncompliant02.py %} ``` For the sake of the example, let's assume we have 5 clients, with 5 accounts and 5 cards for each account `number_of_times = 5`. In that case, we end up calling `check_card_validity() 125` times, exhausting `_max_workers` which is typically at around `32`. [Source: Python Docs] You can additionally call `self.executor._work_queue.qsize()` to get the exact number of tasks that are waiting in the queue, unable to be executed by the already busy workers. @@ -166,69 +45,8 @@ Python, as opposed to Java, does not provide `CallerRunsPolicy`, which was sugge *[compliant02.py](compliant02.py):* -```py -""" Compliant Code Example """ - -from concurrent.futures import ThreadPoolExecutor, wait -from threading import Lock -from typing import Callable - - -class BankingService(object): - def __init__(self, n: int): - self.executor = ThreadPoolExecutor() - self.number_of_times = n - self.count = 0 - self.all_tasks = [] - self.lock = Lock() - - def for_each_client(self): - print("Per client") - self.invoke_method(self.for_each_account) - - def for_each_account(self): - print("Per account") - self.invoke_method(self.for_each_card) - - def for_each_card(self): - print("Per card") - self.invoke_method(self.check_card_validity) - - def check_card_validity(self): - with self.lock: - self.count += 1 - print(f"Number of checked cards: {self.count}") - - def invoke_method(self, method: Callable): - futures = [] - for _ in range(self.number_of_times): - if self.can_fit_in_executor(): - self.lock.acquire() - future = self.executor.submit(method) - self.all_tasks.append(future) - self.lock.release() - futures.append(future) - else: - # Execute the method in the thread that invokes it - method() - wait(futures) - - def can_fit_in_executor(self): - running = 0 - for future in self.all_tasks: - if future.running(): - running += 1 - print(f"Max workers: {self.executor._max_workers}, Used workers: {running}") - # Depending on the order of submitted subtasks, the script can sometimes deadlock - # if we don't leave a spare worker. - return self.executor._max_workers > running + 1 - - -##################### -# exploiting above code example -##################### -browser_manager = BankingService(5) -browser_manager.for_each_client() +```python +{% include_relative compliant02.py %} ``` This compliant code example adds a list of all submitted tasks to the `BankingService` class. The lock, apart from controlling the counter, now ensures that only one thread can access the `all_tasks` list at a time. Before a new task is submitted, `can_fit_in_executor()` checks if submitting a new task will exhaust the thread pool. If so, the task is executed in the thread that tried to submit it. 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..2eec9b11 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 @@ -11,19 +11,8 @@ The `example01.py` code demonstrates bit-wise operators available in Python. *[example01.py](example01.py):* -```py -foo = 50 -bar = 42 -print(f"foo = {foo} = {foo:08b}") # :08b is just for pretty print -print(f"foo = {bar} = {bar:08b}\n") - -# bit wise operations in Python: -print(f"foo << 2 = {(foo << 2):08b}") # binary shift left -print(f"foo >> 2 = {(foo >> 2):08b}") # binary shift right -print(f"~foo = {(~foo):08b}") # flipping bits -print(f"foo & bar = {(foo & bar):08b}") # binary AND -print(f"foo | bar = {(foo | bar):08b}") # binary OR -print(f"foo ^ bar = {(foo ^ bar):08b}") # binary XOR +```python +{% include_relative example01.py %} ``` Output from above example01.py: @@ -31,7 +20,7 @@ Output from above example01.py: ```bash foo = 50 = 00110010 foo = 42 = 00101010 - + foo << 2 = 11001000 foo >> 2 = 00001100 ~foo = -0110011 @@ -44,10 +33,8 @@ The `example02.py` code demonstrates how Python 2 changes an int to long to prev *[example02.py](example02.py):* -```py -for shift in [16, 32, 64]: - bar = 5225 << shift - print("foo << " + str(shift) + ": type " + str(type(bar)) + " " + str(bin(bar))) +```python +{% include_relative example02.py %} ``` Left shift in `example02.py` changes type to long class in Python 2: @@ -72,10 +59,8 @@ Multiplication by `4` can be archived by a `2x` left. The `noncompliant01.py` co *[noncompliant01.py](noncompliant01.py):* -```py -""" Non-compliant Code Example """ - -print(8 << 2 + 10) +```python +{% include_relative noncompliant01.py %} ``` The `noncompliaint01.py` code results in printing `32768` instead of `42`. Adding brackets `print((8 << 2) + 10)` would fix this specific issue whilst still remaining prune to other issues. @@ -86,10 +71,8 @@ The statement in `compliant01.py` clarifies the programmer's intention. *[compliant01.py](compliant01.py):* -```py -""" Compliant Code Example """ - -print(8 * 4 + 10) +```python +{% include_relative compliant01.py %} ``` It is recommended by *[CWE-191, Integer Underflow (Wrap or Wraparound)](../CWE-191/README.md)* to also check for under or overflow. @@ -100,13 +83,8 @@ In this non-compliant code example is using an arithmetic right shift >>= operat *[noncompliant02.py](noncompliant02.py):* -```py -""" Non-compliant Code Example """ - -foo: int -foo = -50 -foo >>= 2 -print(foo) +```python +{% include_relative noncompliant02.py %} ``` The expectation of the `>>= 2` right shift operator is that it fills the leftmost bits of `0011 0010` with two zeros resulting in `0000 1100` or decimal twelve. Instead a potentially expected `-12` in `foo` we have internal processing truncating the values from `-12.5` to `-13`. @@ -117,13 +95,8 @@ The right shift is replaced by division in `compliant02.py`. *[compliant02.py](compliant02.py):* -```py -""" Compliant Code Example """ - -foo: int -foo = -50 -foo /= 4 -print(foo) +```python +{% include_relative compliant02.py %} ``` ## Automated Detection diff --git a/docs/Secure-Coding-Guide-for-Python/CWE-703/CWE-392/README.md b/docs/Secure-Coding-Guide-for-Python/CWE-703/CWE-392/README.md index 41234b18..6fe51aff 100644 --- a/docs/Secure-Coding-Guide-for-Python/CWE-703/CWE-392/README.md +++ b/docs/Secure-Coding-Guide-for-Python/CWE-703/CWE-392/README.md @@ -10,26 +10,8 @@ One of the ways of running a task with `ThreadPoolExecutor` is using a `submit() *[noncompliant01.py](noncompliant01.py):* -```py -""" Non-compliant Code Example """ -import math -from concurrent.futures import ThreadPoolExecutor - - -def get_sqrt(a): - return math.sqrt(a) - - -def run_thread(var): - with ThreadPoolExecutor() as executor: - return executor.submit(get_sqrt, var) - -##################### -# exploiting above code example -##################### -# get_sqrt will raise ValueError that will be suppressed by the ThreadPoolExecutor -arg = -1 -result = run_thread(arg) # The outcome of the task is unknown +```python +{% include_relative noncompliant01.py %} ``` ## Compliant Solution (exception() method) @@ -38,30 +20,8 @@ In order to determine whether an exception occurred, we can call the `exception( *[compliant01.py](compliant01.py):* -```py -""" Compliant Code Example """ -import math -from concurrent.futures import ThreadPoolExecutor - - -def get_sqrt(a): - return math.sqrt(a) - - -def run_thread(var): - with ThreadPoolExecutor() as executor: - future = executor.submit(get_sqrt, var) - if future.exception() is not None: - # handle exception... - raise ValueError(f"Invalid argument: {var}") - return future - - -##################### -# exploiting above code example -##################### -arg = -1 -result = run_thread(arg) # Now code execution will be interrupted by ValueError +```python +{% include_relative compliant01.py %} ``` ## Compliant Solution (result() method) @@ -70,32 +30,8 @@ The exception that was suppressed by the ThreadPoolExecutor will also be raised *[compliant02.py](compliant02.py):* -```py -""" Compliant Code Example """ -import math -from concurrent.futures import ThreadPoolExecutor - - -def get_sqrt(a): - return math.sqrt(a) - - -def run_thread(var): - with ThreadPoolExecutor() as executor: - future = executor.submit(get_sqrt, var) - try: - res = future.result() - return res - except ValueError as e: - # handle exception... - raise ValueError(f"Invalid argument: {var}") from None - - -##################### -# exploiting above code example -##################### -arg = -1 -result = run_thread(arg) # Now code execution will be interrupted by ValueError +```python +{% include_relative compliant02.py %} ``` A similar example of using the `result()` and `exception()` methods to handle exceptions in the Future objects can be found at [[Ross 2020]](https://skeptric.com/python-futures-exception/index.html). @@ -106,31 +42,8 @@ Alternatively, the asynchronous tasks can be run by the ThreadPoolExecutor using *[noncompliant02.py](noncompliant02.py):* -```py -""" Non-compliant Code Example """ - -import math -from concurrent.futures import ThreadPoolExecutor - - -def get_sqrt(a): - return math.sqrt(a) - - -def map_threads(x): - with ThreadPoolExecutor() as executor: - return executor.map(get_sqrt, x) - - -##################### -# exploiting above code example -##################### - -# get_sqrt will raise ValueError that will be suppressed by the ThreadPoolExecutor -args = [2, -1, 4] -results = map_threads(args) -for result in results: - print(result) # Unhandled ValueError will be raised before all results are read +```python +{% include_relative noncompliant02.py %} ``` ## Compliant Solution (Custom exception handling) @@ -139,43 +52,8 @@ Since an exception is raised during iterating over the results, we can handle th *[compliant03.py](compliant03.py):* -```py -""" Compliant Code Example """ - -import math -from concurrent.futures import ThreadPoolExecutor - - -def get_sqrt(a): - return math.sqrt(a) - - -def map_threads(x): - with ThreadPoolExecutor() as executor: - result_gen = executor.map(get_sqrt, x) - ret = list() - invalid_arg = 0 - try: - for res in result_gen: - ret.append(res) - invalid_arg += 1 - return res - except ValueError as e: - # handle exception... - raise ValueError( - f"Invalid argument: {x[invalid_arg]} at list index {invalid_arg}" - ) from None - - -##################### -# exploiting above code example -##################### -args = [2, -1, 4] -results = map_threads(args) -for result in results: - print( - result - ) # The exception is handled, but the rest of the results are still unavailable +```python +{% include_relative compliant03.py %} ``` Even after handling the exception, the results of tasks submitted after the erroneous task are still not available. If we were to iterate over the results manually using the `next()` method, we could see that after the task exception, a StopIterator is also raised. This terminates the result generator's useful life, which means further results will not be generated. This mechanism was described in [[PEP 0255]](https://peps.python.org/pep-0255/#specification-generators-and-exception-propagation). @@ -184,33 +62,8 @@ If we want to make sure all task results will be available, we can either avoid *[compliant04.py](compliant04.py):* -```py -""" Compliant Code Example """ - -import math -from concurrent.futures import ThreadPoolExecutor - - -def get_sqrt(a): - try: - return math.sqrt(a) - except ValueError as e: - print(f"Invalid argument: {a}") - return None - - -def map_threads(x): - with ThreadPoolExecutor() as executor: - return executor.map(get_sqrt, x) - - -##################### -# exploiting above code example -##################### -args = [2, -1, 4] -results = map_threads(args) -for result in results: - print(result) # Now no exception is raised and we can read all of the results +```python +{% include_relative compliant04.py %} ``` More examples of handling exceptions within and outside the concurrent tasks can be found at [[Brownlee 2022]](https://superfastpython.com/threadpoolexecutor-exception-handling/).