Skip to content

Commit 49bdcbc

Browse files
committed
Fixes for the pull request reviews
Signed-off-by: edanhub <[email protected]>
1 parent 681717c commit 49bdcbc

File tree

3 files changed

+61
-38
lines changed

3 files changed

+61
-38
lines changed

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

Lines changed: 51 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,23 @@ Although the individual methods may be thread-safe, that might not be the case w
1010

1111
## Non-Compliant Code Example
1212

13-
The practice of chaining methods is often used in the `Builder` design pattern for setting optional object fields. Values shared fields can become inconsistent during concurrent access as demonstrated in `noncompliant01.py`.
14-
Run `noncomplilant01.py` multiple times to see the effect.
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+
1517
*[noncompliant01.py](noncompliant01.py):*
1618

1719
```python
1820
# SPDX-FileCopyrightText: OpenSSF project contributors
1921
# SPDX-License-Identifier: MIT
2022
"""Noncompliant Code Example"""
21-
23+
2224
from time import sleep
2325
import logging
2426
import threading
25-
26-
27+
import secrets
28+
29+
2730
def thread_function(animal: "Animal", animal_name: str, animal_sound: str):
2831
"""Function that changes animal's characteristics using method chaining"""
2932
for _ in range(3):
@@ -38,49 +41,55 @@ def thread_function(animal: "Animal", animal_name: str, animal_sound: str):
3841
animal.name,
3942
animal.sound,
4043
)
41-
for _ in range(10000000):
42-
pass # Simulate a longer operation on non-shared resources
43-
44-
44+
# Simulate a longer operation on non-shared resources
45+
for i in range(10, 1000):
46+
_ = (secrets.randbelow(i) + 1) / i
47+
48+
4549
class Animal:
4650
"""Class that represents an animal"""
47-
51+
4852
# The characteristics of the animal (optional fields)
4953
def __init__(self):
5054
self.name = ""
5155
self.sound = ""
52-
56+
5357
def set_name(self, name: str):
5458
"""Sets the animal's name"""
5559
self.name = name
5660
# force the thread to lose the lock on the object by
5761
# simulating a long running operation
5862
sleep(0.1)
5963
return self
60-
64+
6165
def set_sound(self, sound: str):
6266
"""Sets the sound that the animal makes"""
6367
self.sound = sound
6468
sleep(0.2)
6569
return self
66-
67-
70+
71+
6872
#####################
6973
# Exploiting above code example
7074
#####################
71-
75+
7276
if __name__ == "__main__":
7377
MESSAGE_FORMAT = "%(asctime)s: %(message)s"
74-
logging.basicConfig(format=MESSAGE_FORMAT, level=logging.INFO, datefmt="%H:%M:%S")
75-
78+
logging.basicConfig(
79+
format=MESSAGE_FORMAT, level=logging.INFO, datefmt="%H:%M:%S"
80+
)
81+
7682
animal = Animal()
7783
dog = threading.Thread(
7884
target=thread_function,
7985
args=(animal, "DOG", "WOOF"),
8086
)
81-
cat = threading.Thread(target=thread_function, args=(animal, "CAT", "MEOW"))
87+
cat = threading.Thread(
88+
target=thread_function, args=(animal, "CAT", "MEOW")
89+
)
8290
dog.start()
8391
cat.start()
92+
8493
```
8594

8695
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.
@@ -95,14 +104,15 @@ This compliant solution uses a lock to ensure that the object cannot be written
95104
# SPDX-FileCopyrightText: OpenSSF project contributors
96105
# SPDX-License-Identifier: MIT
97106
"""Compliant Code Example"""
98-
107+
99108
from time import sleep
100109
import logging
101110
import threading
102-
111+
import secrets
112+
103113
LOCK = threading.Lock()
104-
105-
114+
115+
106116
def thread_function(animal: "Animal", animal_name: str, animal_sound: str):
107117
"""Function that changes animal's characteristics using method chaining"""
108118
for _ in range(3):
@@ -112,48 +122,53 @@ def thread_function(animal: "Animal", animal_name: str, animal_sound: str):
112122
animal.name,
113123
animal.sound,
114124
)
125+
# First time, name and sound will be blank because
126+
# the object isn't initialized yet.
115127
animal.set_name(animal_name).set_sound(animal_sound)
116128
logging.info(
117129
"Thread: finishing - %s goes %s",
118130
animal.name,
119131
animal.sound,
120132
)
121133
LOCK.release()
122-
for _ in range(10000000):
123-
pass # Simulate a longer operation on non-shared resources
124-
125-
134+
# Simulate a longer operation on non-shared resources
135+
for i in range(10, 1000):
136+
_ = (secrets.randbelow(i) + 1) / i
137+
138+
126139
class Animal:
127140
"""Class that represents an animal"""
128-
141+
129142
# The characteristics of the animal (optional fields)
130143
def __init__(self):
131144
self.name = ""
132145
self.sound = ""
133-
146+
134147
def set_name(self, name: str):
135148
"""Sets the animal's name"""
136149
self.name = name
137150
# force the thread to lose the lock on the object by
138151
# simulating a long running operation
139152
sleep(0.1)
140153
return self
141-
154+
142155
def set_sound(self, sound: str):
143156
"""Sets the sound that the animal makes"""
144157
self.sound = sound
145158
sleep(0.2)
146159
return self
147-
148-
160+
161+
149162
#####################
150163
# Exploiting above code example
151164
#####################
152-
165+
153166
if __name__ == "__main__":
154167
MESSAGE_FORMAT = "%(asctime)s: %(message)s"
155-
logging.basicConfig(format=MESSAGE_FORMAT, level=logging.INFO, datefmt="%H:%M:%S")
156-
168+
logging.basicConfig(
169+
format=MESSAGE_FORMAT, level=logging.INFO, datefmt="%H:%M:%S"
170+
)
171+
157172
animal = Animal()
158173
dog = threading.Thread(
159174
target=thread_function,
@@ -164,6 +179,7 @@ if __name__ == "__main__":
164179
)
165180
dog.start()
166181
cat.start()
182+
167183
```
168184

169185
## Automated Detection
@@ -187,3 +203,4 @@ if __name__ == "__main__":
187203
|||
188204
|:---|:---|
189205
|[[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: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from time import sleep
66
import logging
77
import threading
8+
import secrets
89

910
LOCK = threading.Lock()
1011

@@ -18,15 +19,18 @@ def thread_function(animal: "Animal", animal_name: str, animal_sound: str):
1819
animal.name,
1920
animal.sound,
2021
)
22+
# First time, name and sound will be blank because
23+
# the object isn't initialized yet.
2124
animal.set_name(animal_name).set_sound(animal_sound)
2225
logging.info(
2326
"Thread: finishing - %s goes %s",
2427
animal.name,
2528
animal.sound,
2629
)
2730
LOCK.release()
28-
for _ in range(10000000):
29-
pass # Simulate a longer operation on non-shared resources
31+
# Simulate a longer operation on non-shared resources
32+
for i in range(10, 1000):
33+
_ = (secrets.randbelow(i) + 1) / i
3034

3135

3236
class Animal:

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from time import sleep
66
import logging
77
import threading
8+
import secrets
89

910

1011
def thread_function(animal: "Animal", animal_name: str, animal_sound: str):
@@ -21,8 +22,9 @@ def thread_function(animal: "Animal", animal_name: str, animal_sound: str):
2122
animal.name,
2223
animal.sound,
2324
)
24-
for _ in range(10000000):
25-
pass # Simulate a longer operation on non-shared resources
25+
# Simulate a longer operation on non-shared resources
26+
for i in range(10, 1000):
27+
_ = (secrets.randbelow(i) + 1) / i
2628

2729

2830
class Animal:

0 commit comments

Comments
 (0)