generated from ossf/project-template
-
Notifications
You must be signed in to change notification settings - Fork 184
CWE-460 #962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
CWE-460 #962
Changes from 4 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
cbacf64
Create compliant.py
kets99 db57284
Create noncompliant.py
kets99 88bf2a1
Create README.md
kets99 73e1102
Update README.md
kets99 08fbbd6
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/noncomplia…
kets99 7cbeb84
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/README.md
kets99 1e3b2b8
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/compliant.py
kets99 28481ac
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/README.md
kets99 ca47af4
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/README.md
kets99 f290377
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/README.md
kets99 4941976
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/README.md
kets99 d7e18de
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/README.md
kets99 80a4e94
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/README.md
kets99 de75910
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/README.md
kets99 9fe869b
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/README.md
kets99 abb9040
Update README.md
kets99 aeea36e
Update compliant.py for lint compliance
kets99 6ec3b1c
Update README.md
kets99 8764bf2
Update README.md
kets99 1c67894
Update README.md
kets99 c86dd78
Update README.md
kets99 fc0d9da
Update README.md
kets99 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
103 changes: 103 additions & 0 deletions
103
docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/README.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
|
||
# CWE-460: Improper Cleanup on Thrown Exception | ||
|
||
The product does not clean up its state or incorrectly cleans up its state when an exception is thrown, leading to unexpected state or control flow. | ||
|
||
Often, when functions or loops become complicated, some level of resource cleanup is needed throughout execution. Exceptions can disturb the flow of the code and prevent the necessary cleanup from happening. | ||
|
||
A consequence of this is that the code is left in a bad state. | ||
|
||
One of the ways to mitigate this is to make sure that cleanup happens or that you should exit the program. Use throwing exceptions sparsely. | ||
|
||
Another way to mitigate this is to use the ‘with’ statement. It simplifies resource management by automatically handling setup and cleanup tasks. It's commonly used with files, network connections and databases to ensure resources are properly released even if errors occur making your code cleaner. | ||
|
||
## Non-Compliant Code Example | ||
|
||
In the noncompliant.py example, a thread gets locked, but not unlocked due to an exception being thrown before it can be closed. This might lead to the lock remaining closed and inaccessible for further use. | ||
|
||
noncompliant.py: | ||
|
||
``` | ||
# SPDX-FileCopyrightText: OpenSSF project contributors | ||
# SPDX-License-Identifier: MIT | ||
"""Non-compliant Code Example""" | ||
import threading | ||
kets99 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
lock = threading.Lock() | ||
|
||
kets99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
def noncompliant_example(): | ||
kets99 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
# the lock has been acquired for performing a critical operation | ||
lock.acquire() | ||
print("Lock acquired, performing critical operation...") | ||
# simulating an error before it can be released | ||
raise ValueError("Something went wrong!") | ||
lock.release() # This line is never reached due to the exception | ||
|
||
try: | ||
kets99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
noncompliant_example() | ||
kets99 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
except ValueError as e: | ||
print(f"Caught exception: {e}") | ||
|
||
# Next attempt to acquire the lock will block forever — deadlock! | ||
lock.acquire() | ||
print("This will never print because the lock was never released.") | ||
kets99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
``` | ||
|
||
In the above code example, the acquired lock never gets released, as an error gets thrown before it can be released. | ||
|
||
## Compliant Solution | ||
|
||
In compliant01.py we use the with statement to ensure that the lock is released properly even if an error is to occur. | ||
|
||
compliant01.py: | ||
## Compliant Code Example | ||
|
||
``` | ||
# SPDX-FileCopyrightText: OpenSSF project contributors | ||
# SPDX-License-Identifier: MIT | ||
""" Compliant Code Example """ | ||
import threading | ||
|
||
lock = threading.Lock() | ||
|
||
kets99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
def compliant_example(): | ||
with lock: | ||
# the lock has been acquired using the 'with' statement and will be released when the block exits; even if an exception occurs | ||
print("Lock acquired, performing critical operation...") | ||
# raising an exception | ||
raise ValueError("Something went wrong!") | ||
print("Lock released.") | ||
|
||
try: | ||
kets99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
compliant_example() | ||
except ValueError as e: | ||
print(f"Caught exception: {e}") | ||
``` | ||
|
||
### with lock: is shorthand for | ||
|
||
``` | ||
lock.acquire() | ||
try: | ||
... | ||
finally: | ||
lock.release() | ||
kets99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
``` | ||
|
||
It is best practice to use 'with' in such cases as it will make sure the resource gets released even if an exception occurs in the execution. | ||
|
||
|
||
## Automated Detection | ||
|
||
||||| | ||
|:---|:---|:---|:---| | ||
|Tool|Version|Checker|Description| | ||
|
||
## Related Guidelines | ||
|
||
||| | ||
|:---|:---| | ||
|[CWE MITRE Pillar](http://cwe.mitre.org/)|[https://cwe.mitre.org/data/definitions/460.html]| | ||
|
||
|
||
|
21 changes: 21 additions & 0 deletions
21
docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/compliant.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
|
||
# SPDX-FileCopyrightText: OpenSSF project contributors | ||
# SPDX-License-Identifier: MIT | ||
""" Compliant Code Example """ | ||
import threading | ||
|
||
lock = threading.Lock() | ||
|
||
def compliant_example(): | ||
with lock: | ||
# the lock has been acquired using the 'with' statement and will be released when the block exits; even if an exception occurs | ||
print("Lock acquired, performing critical operation...") | ||
# raising an exception | ||
raise ValueError("Something went wrong!") | ||
# This line will not be reached because of the exception above, | ||
print("Lock released.") | ||
|
||
try: | ||
compliant_example() | ||
except ValueError as e: | ||
print(f"Caught exception: {e}") | ||
kets99 marked this conversation as resolved.
Show resolved
Hide resolved
|
24 changes: 24 additions & 0 deletions
24
docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-460/noncompliant.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import threading | ||
|
||
|
||
lock = threading.Lock() | ||
|
||
|
||
def noncompliant_example(): | ||
lock.acquire() | ||
print("Lock acquired, performing critical operation...") | ||
raise ValueError("Something went wrong!") | ||
lock.release() # This line is never reached due to the exception | ||
|
||
|
||
try: | ||
noncompliant_example() | ||
except ValueError as e: | ||
print(f"Caught exception: {e}") | ||
|
||
|
||
# Next attempt to acquire the lock will block forever; as there is a deadlock! | ||
lock.acquire() | ||
print("This will not print because the lock was never released.") | ||
|
||
|
||
kets99 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually make the first sentence something like:
Ensure that your code fully and correctly cleans up its state whenever an exception occurs to avoid unexpected state or control flow.
Rather than taking the actual CWE description. We have plans to change the naming of the rulesets in the future in order to avoid this confusion, but for now, I'd change the first sentence to be "always do this" or "never do this"