Skip to content

Commit beb89db

Browse files
authored
CWE-1095: show that list modification is dangerous (#940)
Before this commit the wording was that modifying list works but is not recommended. But it works as long as no two consecutive elements are deleted, otherwise part of elements is not checked at all without any exceptions raised. Changed README.md, compliant01.py and noncompliant01.py to demonstrate that. Signed-off-by: Kyrylo Yatsenko <[email protected]> Signed-off-by: Helge Wehder <[email protected]>
1 parent 89368a5 commit beb89db

File tree

3 files changed

+18
-11
lines changed

3 files changed

+18
-11
lines changed

docs/Secure-Coding-Guide-for-Python/CWE-710/CWE-1095/README.md

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,29 @@ In-place modification of mutable types such as `list`, `dict`, or `set` that are
66

77
## Non-Compliant Code Example (List)
88

9-
This `noncompliant01.py` example will successfully remove the Bob from `userlist` but this modifies the original list `userlist` and is not recommended.
9+
This `noncompliant01.py` example will remove only one name that starts with `B` despite trying to remove them all without any exception raised:
1010

1111
[*noncompliant01.py:*](noncompliant01.py)
1212

1313
```py
1414
""" Non-compliant Code Example """
15-
userlist = ['Alice', 'Bob', 'Charlie']
15+
userlist = ['Alice', 'Bob', 'Bill', 'Charlie']
1616
print(f'Unmodified list: {userlist}')
1717

1818
for user in userlist:
19-
if user == 'Bob':
19+
if user.startswith('B'):
2020
userlist.remove(user)
2121

2222
print(f'Modified list: {userlist}')
2323
```
2424

25+
Output from above noncompliant01.py:
26+
27+
```bash
28+
Unmodified list: ['Alice', 'Bob', 'Bill', 'Charlie']
29+
Modified list: ['Alice', 'Bill', 'Charlie']
30+
```
31+
2532
## Non-Compliant Code Example (Dict)
2633

2734
This `noncompliant02.py` example attempts to delete a dictionary entry, which will result in a `RuntimeError: Dictionary changed size during iteration error` being thrown.
@@ -63,12 +70,12 @@ The `compliant01.py` solution demonstrates both strategies. The first example cr
6370

6471
```py
6572
""" Compliant Code Example """
66-
userlist = ['Alice', 'Bob', 'Charlie']
73+
userlist = ['Alice', 'Bob', 'Bill', 'Charlie']
6774
print(f'Unmodified list: {userlist}')
6875

6976
# Create a copy
7077
for user in userlist.copy():
71-
if user == 'Bob':
78+
if user.startswith('B'):
7279
userlist.remove(user)
7380

7481
print(f'Modified list: {userlist}')
@@ -80,7 +87,7 @@ print(f'Unmodified list: {userlist2}')
8087
# Create new list
8188
activeusers = []
8289
for user in userlist2:
83-
if user != 'Bob':
90+
if user.startswith('B'):
8491
activeusers.append(user)
8592
print(f'New list: {activeusers}')
8693
```

docs/Secure-Coding-Guide-for-Python/CWE-710/CWE-1095/compliant01.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
# SPDX-FileCopyrightText: OpenSSF project contributors
22
# SPDX-License-Identifier: MIT
33
""" Compliant Code Example """
4-
userlist = ['Alice', 'Bob', 'Charlie']
4+
userlist = ['Alice', 'Bob', 'Bill', 'Charlie']
55
print(f'Unmodified list: {userlist}')
66

77
# Create a copy
88
for user in userlist.copy():
9-
if user == 'Bob':
9+
if user.startswith('B'):
1010
userlist.remove(user)
1111

1212
print(f'Modified list: {userlist}')
@@ -18,6 +18,6 @@
1818
# Create new list
1919
activeusers = []
2020
for user in userlist2:
21-
if user != 'Bob':
21+
if not user.startswith('B'):
2222
activeusers.append(user)
2323
print(f'New list: {activeusers}')
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
# SPDX-FileCopyrightText: OpenSSF project contributors
22
# SPDX-License-Identifier: MIT
33
""" Non-compliant Code Example """
4-
userlist = ['Alice', 'Bob', 'Charlie']
4+
userlist = ['Alice', 'Bob', 'Bill', 'Charlie']
55
print(f'Unmodified list: {userlist}')
66

77
for user in userlist:
8-
if user == 'Bob':
8+
if user.startswith('B'):
99
userlist.remove(user)
1010

1111
print(f'Modified list: {userlist}')

0 commit comments

Comments
 (0)