Skip to content

Commit a868b9d

Browse files
myteronmyteron
authored
Merge pull request #991 from s19110/pyDoc2GitHub_CWE-366-2
CWE-366: Race Condition within a Thread
2 parents 55da6c6 + efc3db1 commit a868b9d

File tree

6 files changed

+430
-1
lines changed

6 files changed

+430
-1
lines changed

‎docs/Secure-Coding-Guide-for-Python/CWE-691/CWE-366/README.md‎

Lines changed: 287 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,287 @@
1+
# CWE-366: Race Condition within a Thread
2+
3+
In multithreaded programming, use synchronization mechanisms, such as locks, to avoid race conditions, which occur when multiple threads access shared resources simultaneously and lead to unpredictable results.
4+
5+
> [!NOTE]
6+
> Prerequisite to understand this page:
7+
> [Intro to multiprocessing and multithreading](../../Intro_to_multiprocessing_and_multithreading/readme.md)
8+
9+
Before Python 3.10, both `direct_add` and `method_calling_add` were at risk of race conditions. After Python 3.10 changed how eval breaking operations are handled [[GH-18334 (2021)](https://github.com/python/cpython/pull/18334)], `direct_add` should not require additional locks while `method_calling_add` might give unpredictable results without them. The `example01.py` code example is demonstrating the issue. Its output will differ depending on the version of Python:
10+
11+
_[example01.py:](example01.py)_
12+
13+
```py
14+
# SPDX-FileCopyrightText: OpenSSF project contributors
15+
# SPDX-License-Identifier: MIT
16+
""" Code Example """
17+
import dis
18+
19+
20+
class Number():
21+
"""
22+
Example of a class where a method calls another method
23+
"""
24+
amount = 100
25+
26+
def direct_add(self):
27+
"""Simulating hard work"""
28+
a = 0
29+
a += self.amount
30+
31+
def method_calling_add(self):
32+
"""Simulating hard work"""
33+
a = 0
34+
a += self.read_amount()
35+
36+
def read_amount(self):
37+
"""Simulating data fetching"""
38+
return self.amount
39+
40+
41+
num = Number()
42+
print("direct_add():")
43+
dis.dis(num.direct_add)
44+
print("method_calling_add():")
45+
dis.dis(num.method_calling_add)
46+
47+
```
48+
49+
When run on Python 3.10.13, output shows that `CALL_METHOD` doesn't appear when calling `direct_add` but it does when `method_calling_add` is called instead:
50+
51+
__Output of example01.py:__
52+
53+
```bash
54+
direct_add():
55+
14 0 LOAD_CONST 1 (0)
56+
2 STORE_FAST 1 (a)
57+
58+
15 4 LOAD_FAST 1 (a)
59+
6 LOAD_FAST 0 (self)
60+
8 LOAD_ATTR 0 (amount)
61+
10 INPLACE_ADD
62+
12 STORE_FAST 1 (a)
63+
14 LOAD_CONST 2 (None)
64+
16 RETURN_VALUE
65+
method_calling_add():
66+
19 0 LOAD_CONST 1 (0)
67+
2 STORE_FAST 1 (a)
68+
69+
20 4 LOAD_FAST 1 (a)
70+
6 LOAD_FAST 0 (self)
71+
8 LOAD_METHOD 0 (read_amount)
72+
10 CALL_METHOD 0
73+
12 INPLACE_ADD
74+
14 STORE_FAST 1 (a)
75+
16 LOAD_CONST 2 (None)
76+
18 RETURN_VALUE
77+
```
78+
79+
An update to Python 3.10 has introduced the change that prevents such issues from occurring under specific condition. The [[GH-18334 (2021)](https://github.com/python/cpython/pull/18334)] change has made it so that the GIL is released and re-aquired only after specific operations as opposed to a certain number of any of them. These operations, called "eval breaking", can be found in the `Python/ceval.c` file and call `CHECK_EVAL_BREAKER()` to check if the interpreter should process pending events, such as releasing GIL to switch threads. They don't include inplace operations, such as `INPLACE_ADD` (called when using the `+=` operator) but they do include `CALL_METHOD`. The `dis` library provides a disassembler for analyzing bytecode operations in specific functions [[Python docs 2025 - dis](https://docs.python.org/3/library/dis.html)].
80+
81+
While both methods might cause race conditions on older versions of Python, only the latter method is risky since Python 3.10. Since Python 3.11, `CALL_FUNCTION` and `CALL_METHOD` have been replaced by a singular `CALL` operation, which is eval breaking as well. [[Python docs 2025 - dis](https://docs.python.org/3/library/dis.html)].
82+
83+
## Non-Compliant Code Example - Unsynchronized Addition/Subtraction
84+
85+
The `noncompliant01.py` code example modifies the value of `amount` by adding and subtracting numerous times. Each of the arithmetic operations is performed by an independent thread [[Python docs 2025 - launching parallel tasks](https://docs.python.org/3.9/library/concurrent.futures.html)]. The expected value once both threads finish their calculations should be `0`.
86+
87+
_[noncompliant01.py](noncompliant01.py):_
88+
89+
```python
90+
# SPDX-FileCopyrightText: OpenSSF project contributors
91+
# SPDX-License-Identifier: MIT
92+
""" Non-compliant Code Example """
93+
import logging
94+
import sys
95+
from threading import Thread
96+
97+
logging.basicConfig(level=logging.INFO)
98+
99+
100+
class Number():
101+
"""
102+
Multithreading incompatible class missing locks.
103+
Issue only occures with more than 1 million repetitions.
104+
"""
105+
value = 0
106+
repeats = 1000000
107+
108+
def add(self):
109+
"""Simulating hard work"""
110+
for _ in range(self.repeats):
111+
logging.debug("Number.add: id=%i int=%s size=%s", id(self.value), self.value, sys.getsizeof(self.value))
112+
self.value += self.read_amount()
113+
114+
def remove(self):
115+
"""Simulating hard work"""
116+
for _ in range(self.repeats):
117+
self.value -= self.read_amount()
118+
119+
def read_amount(self):
120+
""" Simulating reading amount from an external source, i.e. a file, a database, etc. """
121+
return 100
122+
123+
124+
if __name__ == "__main__":
125+
#####################
126+
# exploiting above code example
127+
#####################
128+
number = Number()
129+
logging.info("id=%i int=%s size=%s", id(number.value), number.value, sys.getsizeof(number.value))
130+
add = Thread(target=number.add)
131+
substract = Thread(target=number.remove)
132+
add.start()
133+
substract.start()
134+
135+
logging.info('Waiting for threads to finish...')
136+
add.join()
137+
substract.join()
138+
139+
logging.info("id=%i int=%s size=%s", id(number.value), number.value, sys.getsizeof(number.value))
140+
141+
```
142+
143+
Due to a race condition occurring, the value is never what we expect e.g. `0`. In this example it is `-2609100`.
144+
145+
__Example noncompliant01.py output should show int=0:__
146+
147+
```bash
148+
INFO:root:id=2084074055952 int=0 size=24
149+
INFO:root:Waiting for threads to finish...
150+
INFO:root:id=2084083567824 int=-2609100 size=28
151+
```
152+
153+
## Compliant Solution - Using a Lock
154+
155+
This compliant solution uses a lock to ensure atomicity and visibility. It ensure only one thread at a time has access to and can modify `self.value` [[Python docs 2025 - lock](https://docs.python.org/3.9/library/concurrent.futures.html)]:
156+
157+
_[compliant01.py](compliant01.py):_
158+
159+
```python
160+
# SPDX-FileCopyrightText: OpenSSF project contributors
161+
# SPDX-License-Identifier: MIT
162+
""" Non-compliant Code Example """
163+
import logging
164+
import sys
165+
import threading
166+
from threading import Thread
167+
168+
logging.basicConfig(level=logging.INFO)
169+
170+
171+
class Number():
172+
"""
173+
Multithreading compatible class with locks.
174+
"""
175+
value = 0
176+
repeats = 1000000
177+
178+
def __init__(self):
179+
self.lock = threading.Lock()
180+
181+
def add(self):
182+
"""Simulating hard work"""
183+
for _ in range(self.repeats):
184+
logging.debug("Number.add: id=%i int=%s size=%s", id(self.value), self.value, sys.getsizeof(self.value))
185+
self.lock.acquire()
186+
self.value += self.read_amount()
187+
self.lock.release()
188+
189+
def remove(self):
190+
"""Simulating hard work"""
191+
for _ in range(self.repeats):
192+
self.lock.acquire()
193+
self.value -= self.read_amount()
194+
self.lock.release()
195+
196+
def read_amount(self):
197+
""" Simulating reading amount from an external source, i.e. a file, a database, etc. """
198+
return 100
199+
200+
201+
if __name__ == "__main__":
202+
#####################
203+
# exploiting above code example
204+
#####################
205+
number = Number()
206+
logging.info("id=%i int=%s size=%s", id(number.value), number.value, sys.getsizeof(number.value))
207+
add = Thread(target=number.add)
208+
substract = Thread(target=number.remove)
209+
add.start()
210+
substract.start()
211+
212+
logging.info('Waiting for threads to finish...')
213+
add.join()
214+
substract.join()
215+
216+
logging.info("id=%i int=%s size=%s", id(number.value), number.value, sys.getsizeof(number.value))
217+
218+
```
219+
220+
__Example compliant01.py output provides the expected output of int=0:__
221+
222+
```bash
223+
INFO:root:id=2799840487696 int=0 size=24
224+
INFO:root:Waiting for threads to finish...
225+
INFO:root:id=2799840487696 int=0 size=24
226+
```
227+
228+
## Automated Detection
229+
230+
<table>
231+
<hr>
232+
<td>Tool</td>
233+
<td>Version</td>
234+
<td>Checker</td>
235+
<td>Description</td>
236+
</hr>
237+
<tr>
238+
<td>Bandit</td>
239+
<td>1.7.4 on Python 3.10.13</td>
240+
<td>Not Available</td>
241+
<td></td>
242+
</tr>
243+
<tr>
244+
<td>Flake8</td>
245+
<td>8-4.0.1 on Python 3.10.13</td>
246+
<td>Not Available</td>
247+
<td></td>
248+
</tr>
249+
</table>
250+
251+
## Related Guidelines
252+
253+
<table>
254+
<tr>
255+
<td><a href="http://cwe.mitre.org/">MITRE CWE</a></td>
256+
<td>Pillar: <a href="https://cwe.mitre.org/data/definitions/691.html"> [CWE-691: Insufficient Control Flow Management]</a></td>
257+
</tr>
258+
<tr>
259+
<td><a href="http://cwe.mitre.org/">MITRE CWE</a></td>
260+
<td>Base: <a href="https://cwe.mitre.org/data/definitions/366.html">[CWE-366: Race Condition within a Thread (4.18)]</a></td>
261+
</tr>
262+
<tr>
263+
<td><a href="https://wiki.sei.cmu.edu/confluence/display/java/SEI+CERT+Oracle+Coding+Standard+for+Java">SEI CERT Oracle Coding Standard for Java</a></td>
264+
<td><a href="https://wiki.sei.cmu.edu/confluence/display/java/VNA02-J.+Ensure+that+compound+operations+on+shared+variables+are+atomic">[VNA02-J. Ensure that compound operations on shared variables are atomic]</a></td>
265+
</tr>
266+
</table>
267+
268+
## Bibliography
269+
270+
<table>
271+
<tr>
272+
<td>[Python docs 2025 - launching parallel tasks]</td>
273+
<td>Python Software Foundation. (2024). concurrent.futures — Launching parallel tasks [online]. Available from: <a href="https://docs.python.org/3.10/library/concurrent.futures.html">https://docs.python.org/3.10/library/concurrent.futures.html</a>, [Accessed 18 September 2025]</td>
274+
</tr>
275+
<tr>
276+
<td>[Python docs 2025 - lock]</td>
277+
<td>Python Software Foundation. (2024). Lock Objects [online]. Available from: <a href="https://docs.python.org/3.10/library/threading.html#lock-objects">https://docs.python.org/3.10/library/threading.html#lock-objects</a>, [Accessed 18 September 2025]</td>
278+
</tr>
279+
<tr>
280+
<td>[Python docs 2025 - dis]</td>
281+
<td>Python Software Foundation. (2024). dis — Disassembler for Python bytecode [online]. Available from: <a href="https://docs.python.org/3/library/dis.html">https://docs.python.org/3/library/dis.html</a>, [Accessed 18 September 2025]</td>
282+
</tr>
283+
<tr>
284+
<td>[GH-18334 (2021)]</td>
285+
<td>GitHub CPython bpo-29988: Only check evalbreaker after calls and on backwards egdes. #18334 [online]. Available from: <a href="https://github.com/python/cpython/pull/18334">https://github.com/python/cpython/pull/18334</a>, [Accessed 18 September 2025]</td>
286+
</tr>
287+
</table>

‎docs/Secure-Coding-Guide-for-Python/CWE-691/CWE-366/compliant01.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# SPDX-FileCopyrightText: OpenSSF project contributors
2+
# SPDX-License-Identifier: MIT
3+
""" Non-compliant Code Example """
4+
import logging
5+
import sys
6+
import threading
7+
from threading import Thread
8+
9+
logging.basicConfig(level=logging.INFO)
10+
11+
12+
class Number():
13+
"""
14+
Multithreading compatible class with locks.
15+
"""
16+
value = 0
17+
repeats = 1000000
18+
19+
def __init__(self):
20+
self.lock = threading.Lock()
21+
22+
def add(self):
23+
"""Simulating hard work"""
24+
for _ in range(self.repeats):
25+
logging.debug("Number.add: id=%i int=%s size=%s", id(self.value), self.value, sys.getsizeof(self.value))
26+
self.lock.acquire()
27+
self.value += self.read_amount()
28+
self.lock.release()
29+
30+
def remove(self):
31+
"""Simulating hard work"""
32+
for _ in range(self.repeats):
33+
self.lock.acquire()
34+
self.value -= self.read_amount()
35+
self.lock.release()
36+
37+
def read_amount(self):
38+
""" Simulating reading amount from an external source, i.e. a file, a database, etc. """
39+
return 100
40+
41+
42+
if __name__ == "__main__":
43+
#####################
44+
# exploiting above code example
45+
#####################
46+
number = Number()
47+
logging.info("id=%i int=%s size=%s", id(number.value), number.value, sys.getsizeof(number.value))
48+
add = Thread(target=number.add)
49+
substract = Thread(target=number.remove)
50+
add.start()
51+
substract.start()
52+
53+
logging.info('Waiting for threads to finish...')
54+
add.join()
55+
substract.join()
56+
57+
logging.info("id=%i int=%s size=%s", id(number.value), number.value, sys.getsizeof(number.value))

‎docs/Secure-Coding-Guide-for-Python/CWE-691/CWE-366/example01.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# SPDX-FileCopyrightText: OpenSSF project contributors
2+
# SPDX-License-Identifier: MIT
3+
""" Code Example """
4+
import dis
5+
6+
7+
class Number():
8+
"""
9+
Example of a class where a method calls another method
10+
"""
11+
amount = 100
12+
13+
def direct_add(self):
14+
"""Simulating hard work"""
15+
a = 0
16+
a += self.amount
17+
18+
def method_calling_add(self):
19+
"""Simulating hard work"""
20+
a = 0
21+
a += self.read_amount()
22+
23+
def read_amount(self):
24+
"""Simulating data fetching"""
25+
return self.amount
26+
27+
28+
num = Number()
29+
print("direct_add():")
30+
dis.dis(num.direct_add)
31+
print("method_calling_add():")
32+
dis.dis(num.method_calling_add)

0 commit comments

Comments
 (0)