Skip to content

Commit 76d1126

Browse files
s19110myteronandrew-costello
authored
pySCG: Adding documentation to CWE-362 as part of #531 (#815)
* pySCG: Adding documentation to CWE-362 as part of #531 --------- Signed-off-by: edanhub <[email protected]> Signed-off-by: Hubert Daniszewski <[email protected]> Co-authored-by: myteron <[email protected]> Co-authored-by: andrew-costello <[email protected]>
1 parent a451c6f commit 76d1126

File tree

4 files changed

+357
-0
lines changed

4 files changed

+357
-0
lines changed

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

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
# CWE-362: Concurrent Execution Using Shared Resource with Improper Synchronization ("Race Condition")
2+
3+
Ensure to implement locking mechanisms when chaining methods in a multithreaded environment to prevent unexpected results.
4+
5+
Method chaining is a programming technique where multiple methods are called on the same object sequentially, with each method call returning the object itself or another object that supports further method calls.  Objects that return a reference to themselves allow method chaining, which we frequently use when stripping strings of unwanted content:
6+
7+
> `"Hello There\n".lstrip().rstrip()`
8+
9+
Although the individual methods may be thread-safe, that might not be the case when they are chained together.
10+
11+
## Non-Compliant Code Example
12+
13+
The practice of chaining methods is often used in the `Builder` design pattern for setting optional object fields [Bloch 2017]. Values shared fields can become inconsistent during concurrent access as demonstrated in `noncompliant01.py`.
14+
15+
Since the order of threads may differ between runs, you might need to run `noncomplilant01.py` multiple times to see the effect.
16+
17+
*[noncompliant01.py](noncompliant01.py):*
18+
19+
```python
20+
# SPDX-FileCopyrightText: OpenSSF project contributors
21+
# SPDX-License-Identifier: MIT
22+
"""Noncompliant Code Example"""
23+
24+
from time import sleep
25+
import logging
26+
import threading
27+
import secrets
28+
29+
30+
def thread_function(animal: "Animal", animal_name: str, animal_sound: str):
31+
"""Function that changes animal's characteristics using method chaining"""
32+
for _ in range(3):
33+
logging.info(
34+
"Thread: starting - %s goes %s",
35+
animal.name,
36+
animal.sound,
37+
)
38+
animal.set_name(animal_name).set_sound(animal_sound)
39+
logging.info(
40+
"Thread: finishing - %s goes %s",
41+
animal.name,
42+
animal.sound,
43+
)
44+
# Simulate a longer operation on non-shared resources
45+
for i in range(10, 1000):
46+
_ = (secrets.randbelow(i) + 1) / i
47+
48+
49+
class Animal:
50+
"""Class that represents an animal"""
51+
52+
# The characteristics of the animal (optional fields)
53+
def __init__(self):
54+
self.name = ""
55+
self.sound = ""
56+
57+
def set_name(self, name: str):
58+
"""Sets the animal's name"""
59+
self.name = name
60+
# force the thread to lose the lock on the object by
61+
# simulating a long running operation
62+
sleep(0.1)
63+
return self
64+
65+
def set_sound(self, sound: str):
66+
"""Sets the sound that the animal makes"""
67+
self.sound = sound
68+
sleep(0.2)
69+
return self
70+
71+
72+
#####################
73+
# Exploiting above code example
74+
#####################
75+
76+
if __name__ == "__main__":
77+
MESSAGE_FORMAT = "%(asctime)s: %(message)s"
78+
logging.basicConfig(
79+
format=MESSAGE_FORMAT, level=logging.INFO, datefmt="%H:%M:%S"
80+
)
81+
82+
animal = Animal()
83+
dog = threading.Thread(
84+
target=thread_function,
85+
args=(animal, "DOG", "WOOF"),
86+
)
87+
cat = threading.Thread(
88+
target=thread_function, args=(animal, "CAT", "MEOW")
89+
)
90+
dog.start()
91+
cat.start()
92+
93+
```
94+
95+
In `noncompliant01.py` , the client constructs an `Animal` object and runs two threads. One of the threads is trying to create a dog while the other thread sets up a cat. The expected result of this code example is for the animal to always have the desired set of characteristics. The [CPython Global Interpreter Lock(GIL)](https://docs.python.org/3/glossary.html#term-global-interpreter-lock) does not prevent unexpected results in this case. Sometimes, the code may result in a meowing dog or a barking cat.
96+
97+
## Compliant Solution
98+
99+
This compliant solution uses a lock to ensure that the object cannot be written to while another thread is using it.
100+
101+
*[compliant01.py](compliant01.py):*
102+
103+
```python
104+
# SPDX-FileCopyrightText: OpenSSF project contributors
105+
# SPDX-License-Identifier: MIT
106+
"""Compliant Code Example"""
107+
108+
from time import sleep
109+
import logging
110+
import threading
111+
import secrets
112+
113+
LOCK = threading.Lock()
114+
115+
116+
def thread_function(animal: "Animal", animal_name: str, animal_sound: str):
117+
"""Function that changes animal's characteristics using method chaining"""
118+
for _ in range(3):
119+
LOCK.acquire()
120+
logging.info(
121+
"Thread: starting - %s goes %s",
122+
animal.name,
123+
animal.sound,
124+
)
125+
# First time, name and sound will be blank because
126+
# the object isn't initialized yet.
127+
animal.set_name(animal_name).set_sound(animal_sound)
128+
logging.info(
129+
"Thread: finishing - %s goes %s",
130+
animal.name,
131+
animal.sound,
132+
)
133+
LOCK.release()
134+
# Simulate a longer operation on non-shared resources
135+
for i in range(10, 1000):
136+
_ = (secrets.randbelow(i) + 1) / i
137+
138+
139+
class Animal:
140+
"""Class that represents an animal"""
141+
142+
# The characteristics of the animal (optional fields)
143+
def __init__(self):
144+
self.name = ""
145+
self.sound = ""
146+
147+
def set_name(self, name: str):
148+
"""Sets the animal's name"""
149+
self.name = name
150+
# force the thread to lose the lock on the object by
151+
# simulating a long running operation
152+
sleep(0.1)
153+
return self
154+
155+
def set_sound(self, sound: str):
156+
"""Sets the sound that the animal makes"""
157+
self.sound = sound
158+
sleep(0.2)
159+
return self
160+
161+
162+
#####################
163+
# Exploiting above code example
164+
#####################
165+
166+
if __name__ == "__main__":
167+
MESSAGE_FORMAT = "%(asctime)s: %(message)s"
168+
logging.basicConfig(
169+
format=MESSAGE_FORMAT, level=logging.INFO, datefmt="%H:%M:%S"
170+
)
171+
172+
animal = Animal()
173+
dog = threading.Thread(
174+
target=thread_function,
175+
args=(animal, "DOG", "WOOF"),
176+
)
177+
cat = threading.Thread(
178+
target=thread_function, args=(animal, "CAT", "MEOW")
179+
)
180+
dog.start()
181+
cat.start()
182+
183+
```
184+
185+
## Automated Detection
186+
187+
|Tool|Version|Checker|Description|
188+
|:---|:---|:---|:---|
189+
|Bandit|1.7.4 on Python 3.10.4|Not Available||
190+
|Flake8|8-4.0.1 on Python 3.10.4|Not Available||
191+
192+
## Related Guidelines
193+
194+
|||
195+
|:---|:---|
196+
|[MITRE CWE](http://cwe.mitre.org/)|Pillar: [CWE-691: Insufficient Control Flow Management (4.13) (mitre.org)](https://cwe.mitre.org/data/definitions/691.html)|
197+
|[MITRE CWE](http://cwe.mitre.org/)|Class: [CWE-362: Concurrent Execution Using Shared Resource with Improper Synchronization ("Race Condition")](https://cwe.mitre.org/data/definitions/362.html)|
198+
|[MITRE CWE](http://cwe.mitre.org/)|Class: [CWE-662: Numeric Truncation Error](https://cwe.mitre.org/data/definitions/662.html)|
199+
|[MITRE CWE](http://cwe.mitre.org/)|Base: [CWE-366: Race Condition within a Thread](https://cwe.mitre.org/data/definitions/366.html)|
200+
201+
## Bibliography
202+
203+
|||
204+
|:---|:---|
205+
|[[Python docs](https://docs.python.org/3/library/threading.html)]|Python Software Foundation. (2024). threading — Thread-based parallelism [online]. Available from: [https://docs.python.org/3/library/threading.html](https://docs.python.org/3/library/threading.html) [accessed 18 March 2024]|
206+
|[Bloch 2017]|Bloch, J. (2017) Creating and Destroying Objects. In: Friendly, S. and Lindholm, T. and Hendrickson, M., eds. Effective Java. 3rd ed. Boston: Addison-Wesley Professional, pp.10-17.|

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# SPDX-FileCopyrightText: OpenSSF project contributors
2+
# SPDX-License-Identifier: MIT
3+
"""Compliant Code Example"""
4+
5+
from time import sleep
6+
import logging
7+
import threading
8+
import secrets
9+
10+
LOCK = threading.Lock()
11+
12+
13+
def thread_function(animal: "Animal", animal_name: str, animal_sound: str):
14+
"""Function that changes animal's characteristics using method chaining"""
15+
for _ in range(3):
16+
LOCK.acquire()
17+
logging.info(
18+
"Thread: starting - %s goes %s",
19+
animal.name,
20+
animal.sound,
21+
)
22+
# First time, name and sound will be blank because
23+
# the object isn't initialized yet.
24+
animal.set_name(animal_name).set_sound(animal_sound)
25+
logging.info(
26+
"Thread: finishing - %s goes %s",
27+
animal.name,
28+
animal.sound,
29+
)
30+
LOCK.release()
31+
# Simulate a longer operation on non-shared resources
32+
for i in range(10, 1000):
33+
_ = (secrets.randbelow(i) + 1) / i
34+
35+
36+
class Animal:
37+
"""Class that represents an animal"""
38+
39+
# The characteristics of the animal (optional fields)
40+
def __init__(self):
41+
self.name = ""
42+
self.sound = ""
43+
44+
def set_name(self, name: str):
45+
"""Sets the animal's name"""
46+
self.name = name
47+
# force the thread to lose the lock on the object by
48+
# simulating a long running operation
49+
sleep(0.1)
50+
return self
51+
52+
def set_sound(self, sound: str):
53+
"""Sets the sound that the animal makes"""
54+
self.sound = sound
55+
sleep(0.2)
56+
return self
57+
58+
59+
#####################
60+
# Exploiting above code example
61+
#####################
62+
63+
if __name__ == "__main__":
64+
MESSAGE_FORMAT = "%(asctime)s: %(message)s"
65+
logging.basicConfig(
66+
format=MESSAGE_FORMAT, level=logging.INFO, datefmt="%H:%M:%S"
67+
)
68+
69+
animal = Animal()
70+
dog = threading.Thread(
71+
target=thread_function,
72+
args=(animal, "DOG", "WOOF"),
73+
)
74+
cat = threading.Thread(
75+
target=thread_function, args=(animal, "CAT", "MEOW")
76+
)
77+
dog.start()
78+
cat.start()

‎docs/Secure-Coding-Guide-for-Python/CWE-691/CWE-362/noncompliant01.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# SPDX-FileCopyrightText: OpenSSF project contributors
2+
# SPDX-License-Identifier: MIT
3+
"""Noncompliant Code Example"""
4+
5+
from time import sleep
6+
import logging
7+
import threading
8+
import secrets
9+
10+
11+
def thread_function(animal: "Animal", animal_name: str, animal_sound: str):
12+
"""Function that changes animal's characteristics using method chaining"""
13+
for _ in range(3):
14+
logging.info(
15+
"Thread: starting - %s goes %s",
16+
animal.name,
17+
animal.sound,
18+
)
19+
animal.set_name(animal_name).set_sound(animal_sound)
20+
logging.info(
21+
"Thread: finishing - %s goes %s",
22+
animal.name,
23+
animal.sound,
24+
)
25+
# Simulate a longer operation on non-shared resources
26+
for i in range(10, 1000):
27+
_ = (secrets.randbelow(i) + 1) / i
28+
29+
30+
class Animal:
31+
"""Class that represents an animal"""
32+
33+
# The characteristics of the animal (optional fields)
34+
def __init__(self):
35+
self.name = ""
36+
self.sound = ""
37+
38+
def set_name(self, name: str):
39+
"""Sets the animal's name"""
40+
self.name = name
41+
# force the thread to lose the lock on the object by
42+
# simulating a long running operation
43+
sleep(0.1)
44+
return self
45+
46+
def set_sound(self, sound: str):
47+
"""Sets the sound that the animal makes"""
48+
self.sound = sound
49+
sleep(0.2)
50+
return self
51+
52+
53+
#####################
54+
# Exploiting above code example
55+
#####################
56+
57+
if __name__ == "__main__":
58+
MESSAGE_FORMAT = "%(asctime)s: %(message)s"
59+
logging.basicConfig(
60+
format=MESSAGE_FORMAT, level=logging.INFO, datefmt="%H:%M:%S"
61+
)
62+
63+
animal = Animal()
64+
dog = threading.Thread(
65+
target=thread_function,
66+
args=(animal, "DOG", "WOOF"),
67+
)
68+
cat = threading.Thread(
69+
target=thread_function, args=(animal, "CAT", "MEOW")
70+
)
71+
dog.start()
72+
cat.start()

‎docs/Secure-Coding-Guide-for-Python/readme.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ It is __not production code__ and requires code-style or python best practices t
7171

7272
|[CWE-691: Insufficient Control Flow Management](https://cwe.mitre.org/data/definitions/691.html)|Prominent CVE|
7373
|:---------------------------------------------------------------------------------------------------------------|:----|
74+
|[CWE-362: Concurrent Execution Using Shared Resource with Improper Synchronization ("Race Condition")](CWE-691/CWE-362/README.md)||
7475
|[CWE-617: Reachable Assertion](CWE-691/CWE-617/README.md)||
7576

7677
|[CWE-693: Protection Mechanism Failure](https://cwe.mitre.org/data/definitions/693.html)|Prominent CVE|

0 commit comments

Comments
 (0)