generated from ossf/project-template
-
Notifications
You must be signed in to change notification settings - Fork 150
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
CWE-390: Detection of Error Condition without Action #805
Open
BartyBoi1128
wants to merge
3
commits into
ossf:main
Choose a base branch
from
BartyBoi1128:CWE-390
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
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
149 changes: 149 additions & 0 deletions
149
docs/Secure-Coding-Guide-for-Python/CWE-703/CWE-390/REAME.md
This file contains 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,149 @@ | ||
# CWE-390: Detection of Error Condition without Action | ||
|
||
Allow exceptions to bubble up and handle exceptions at the right level in the stack. | ||
|
||
Each `except` block must ensure that the program continues only with formally specified behavior by either: | ||
|
||
* Recovering from the exceptional condition | ||
* Re-throwing the exception with additional information | ||
* Throwing custom exception only if it provides a benefit over Python's [Built-in Exceptions](https://docs.python.org/3.9/library/exceptions.html) [Python.org 2022]. | ||
|
||
Invalid reasons for suppressing exceptions cause: | ||
|
||
* Excessive complexity | ||
* Print statements from lower levels | ||
* Incomplete trace-logs | ||
* Excessive logging | ||
|
||
Printing the stack trace can reveal details and sensitive data about an application such as the components on use, existing users, and other sensitive information such as keys or passwords, as described in [CWE-209: Generation of Error Message Containing Sensitive Information](https://eteamspace.internal.ericsson.com/x/Yr8YS) and will not be handled in these examples. | ||
|
||
## Non-Compliant Code Example - Bare Exception | ||
|
||
In Python `Exception` extends from `BaseException`and a bare `except` will catch everything. | ||
|
||
For instance, catching a bare `except` causes a user to be unable to stop a script via `ctrl+c`, due to the base `except` cathing all exceptions. In comparison, catching `except Exception` allows a `KeyboardInterrupt` to be the Python interpreter itself or other parts of the code. This is due to `KeyboardInterrupt` extending `BaseException` and not `Exception`. | ||
|
||
Note that using `except Exception` is still too broad as per [CWE-755: Improper Handling of Exceptional Conditions](https://github.com/ossf/wg-best-practices-os-developers/blob/main/docs/Secure-Coding-Guide-for-Python/CWE-703/CWE-755/README.md) and that a more specific exception handling is preferred. | ||
|
||
The `noncompliant01.py` code demonstrates a bare except on a `ZeroDivisionError` and must be run on the command line in order to experience the issue. | ||
|
||
*[noncompliant01.py](noncompliant01.py):* | ||
|
||
```py | ||
# SPDX-FileCopyrightText: OpenSSF project contributors | ||
# SPDX-License-Identifier: MIT | ||
""" Non-compliant Code Example """ | ||
|
||
from time import sleep | ||
|
||
|
||
def exception_example(): | ||
"""Non-compliant Code Example using bare except""" | ||
while True: | ||
try: | ||
sleep(1) | ||
_ = 1 / 0 | ||
except: | ||
print("Don't care") | ||
|
||
|
||
##################### | ||
# exploiting above code example | ||
##################### | ||
exception_example() | ||
``` | ||
|
||
The `noncomplaint01.py` will continue to run when launched via terminal even when using `CTRL+C`. | ||
|
||
The process will have to be terminated or killed in order to stop it. A programming IDE will allow stopping the `noncompliant01.py`as IDEs tend to kill the process rather than sending `CTRL+C`. | ||
|
||
## Compliant Code Example - Bare Exception | ||
|
||
The `compliant01.py` code example can be stopped via `CTRL+C` on the command line as it is catching the self created `ZeroDivisionError` instead of using a bare exception. | ||
|
||
*[compliant01.py](compliant01.py):* | ||
|
||
```py | ||
# SPDX-FileCopyrightText: OpenSSF project contributors | ||
# SPDX-License-Identifier: MIT | ||
""" Compliant Code Example """ | ||
from time import sleep | ||
|
||
|
||
def exception_example(): | ||
"""Compliant Code Example catching a specific exception""" | ||
while True: | ||
sleep(1) | ||
try: | ||
_ = 1 / 0 | ||
except ZeroDivisionError: | ||
print("How is it now?") | ||
|
||
|
||
##################### | ||
# exploiting above code example | ||
##################### | ||
exception_example() | ||
``` | ||
|
||
## Exceptions | ||
|
||
The following two exceptions, highlighted in [SEI Cert's Oracle Coding Standard for Java](https://wiki.sei.cmu.edu/confluence/display/java/SEI+CERT+Oracle+Coding+Standard+for+Java), are important to understand when to attempt to handle exceptions at the right level in the stack in Python also. | ||
|
||
We wrote a code example in Python in order to assist in the understanding of these exceptions. | ||
|
||
__ERR00-J-EX0:__ You may suppress exceptions during the release of non-reusable resources, such as closing files, network sockets, or shutting down threads, if they don't affect future program behavior. | ||
__ERR00-J-EX1:__ Allow higher-level code to catch and attempt recovery from exceptions. If recovery is not possible, log the exception, add information if needed, and rethrow it. | ||
|
||
*[example01.py](example01.py):* | ||
|
||
```py | ||
# SPDX-FileCopyrightText: OpenSSF project contributors | ||
# SPDX-License-Identifier: MIT | ||
""" Compliant Code Example """ | ||
from time import sleep | ||
|
||
|
||
def exception_example(): | ||
"""Compliant Code Example catching a specific exception""" | ||
while True: | ||
sleep(1) | ||
try: | ||
_ = 1 / 0 | ||
except ZeroDivisionError: | ||
print("How is it now?") | ||
|
||
|
||
##################### | ||
# exploiting above code example | ||
##################### | ||
exception_example() | ||
``` | ||
|
||
If recovery remains impossible, wrap the checked exception in an unchecked exception and rethrow it. | ||
|
||
## Automated Detection | ||
|
||
|Tool|Version|Checker|Description| | ||
|:----|:----|:----|:----| | ||
|[Ruff](https://docs.astral.sh/ruff/)|v0.4.5|[bare-except (E722)](https://docs.astral.sh/ruff/rules/bare-except/)|Use lazy % formatting in logging functions| | ||
|[Pylint](https://pylint.pycqa.org/)|3.2.7|[W0702:bare-except](https://pylint.pycqa.org/en/latest/user_guide/messages/warning/bare-except.html)|No exception type(s) specified| | ||
|[flake8](https://www.flake8rules.com/)|7.1.1|[E722](https://www.flake8rules.com/rules/E722.html)|do not use bare 'except'| | ||
|
||
## Related Guidelines | ||
|
||
||| | ||
|:---|:---| | ||
|[Secure Coding One Stop Shop for Python](https://github.com/ossf/wg-best-practices-os-developers/tree/main/docs/Secure-Coding-Guide-for-Python)|[CWE-209: Generation of Error Message Containing Sensitive Information](https://eteamspace.internal.ericsson.com/x/Yr8YS)| | ||
|[SEI CERT Oracle Coding Standard for Java](https://wiki.sei.cmu.edu/confluence/display/java/SEI+CERT+Oracle+Coding+Standard+for+Java)|[ERR00-J. Do not suppress or ignore checked exceptions](https://wiki.sei.cmu.edu/confluence/display/java/ERR00-J.+Do+not+suppress+or+ignore+checked+exceptions)| | ||
|[MITRE CWE Base](http://cwe.mitre.org/)|[CWE-703](https://cwe.mitre.org/data/definitions/703.html), Improper Check or Handling of Exceptional Conditions| | ||
|[MITRE CWE Pillar](http://cwe.mitre.org/)|[CWE-390](http://cwe.mitre.org/data/definitions/390.html), Detection of Error Condition without Action| | ||
|
||
## Biblography | ||
|
||
||| | ||
|:---|:---| | ||
|[[Python.org](https://docs.python.org/3.9/) 2022]| python.org. (2022). Built-in Exceptions [online]. Available from: [Python Built-in Exception Documents](https://docs.python.org/3.9/library/exceptions.html) [accessed 08 February 2023]| | ||
|[[Bloch 2008](https://wiki.sei.cmu.edu/confluence/display/java/Rule+AA.+References#RuleAA.References-Bloch08)]|Item 62, "Document All Exceptions Thrown by Each Method"| | ||
|[[Bloch 2008](https://wiki.sei.cmu.edu/confluence/display/java/Rule+AA.+References#RuleAA.References-Bloch08)]| Item 65, "Don't Ignore Exceptions"| | ||
|[[Goetz](https://wiki.sei.cmu.edu/confluence/display/java/Rule+AA.+References#RuleAA.References-Goetz06)]|Section 5.4, "Blocking and Interruptible Methods"| | ||
Comment on lines
+147
to
+149
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bloch and Goetz are not referenced in this doc, contain links to a CMU ref list, have wrong formatting. suggest to remove them.
|
28 changes: 0 additions & 28 deletions
28
docs/Secure-Coding-Guide-for-Python/CWE-703/CWE-390/compliant02.py
This file was deleted.
Oops, something went wrong.
22 changes: 0 additions & 22 deletions
22
docs/Secure-Coding-Guide-for-Python/CWE-703/CWE-390/noncompliant02.py
This file was deleted.
Oops, something went wrong.
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.
on second thought its confusing to have example01 code below exceptions section. Can we move the example01 above the exaceptions sectoin and "If recovery remains impossible, wrap the checked exception in an unchecked exception and rethrow it. " from after the code to before the code as an introduction to why its there?