generated from ossf/project-template
-
Notifications
You must be signed in to change notification settings - Fork 185
adding CWE-459 #893
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
adding CWE-459 #893
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
72b127b
adding CWE-459
dwiley258 17e1da2
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-459/README.md
dwiley258 08552db
Apply suggestions from code review
dwiley258 6f4bb55
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-459/README.md
dwiley258 a25bce7
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-459/compliant0…
dwiley258 128f101
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-459/noncomplia…
dwiley258 6b84f9b
Update docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-459/noncomplia…
dwiley258 cd05ebb
fixed linting errors
myteron 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
91 changes: 91 additions & 0 deletions
91
docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-459/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,91 @@ | ||
| # CWE-459: Incomplete Cleanup | ||
|
|
||
| Leftover temporary files not properly cleaned up after the completion of any script, can lead to resource exhaustion and disable a service. | ||
|
|
||
| Temporary files can be used to store data, either between processes, or to preserve memory. Using temporary files comes with several challenges, we need to ensure that the temporary files are removed before the termination of the process. In the case of the process terminating abnormally, we can't rely on a finally block as it could fail to run. In the case that the program still fails to free up the execution, we need to ensure that the temporary files are created with the correct permissions and in the correct location so the OS can cleanup after the process finishes, while ensuring to restrict access to other processes. | ||
|
|
||
| In Python there is two documented ways to create temporary files using the tempfile library, tempfile.mkstemp() and tempfile.NamedTemporaryFile() . | ||
|
|
||
| tempfile.mksdir() creates a secure file in the most secure fashion allowing only read and write to the user who executed the python script. It returns a tuple, which does not work well with the "with" statement. This mean that the user is responsible for deleting the temporary file after use. | ||
|
|
||
| tempfile.NamedTemporaryFile() is more advanced than the mkstemp() method as it returns a file-like object that works well with the "with" statement, although it creates the file with the same permissions as mkstemp(). The default behaviour is to delete the file once the "with" block is finished. If the file is needed outside of the with block, the delete_on_close parameter must be set to false. | ||
dwiley258 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| ## Non-Compliant Code Example | ||
|
|
||
| In the noncompliant01.py example, a temporary file is created but is not removed after completion. | ||
|
|
||
| *[noncompliant01.py](noncompliant01.py):* | ||
|
|
||
| ```python | ||
| """Non-compliant Code Example""" | ||
|
|
||
dwiley258 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| # SPDX-FileCopyrightText: OpenSSF project contributors | ||
| # SPDX-License-Identifier: MIT | ||
| """ Non-compliant Code Example """ | ||
| f = open("tempfile.txt", "w") | ||
| f.write("temporary file created!") | ||
| f.close() | ||
| ``` | ||
|
|
||
| In noncompliant02.py, we are using the mkstemp method to generate the temporary file. This will create a more secure temporary file but doesn't do clean-up of the file after the end of execution. | ||
|
|
||
| *[noncompliant02.py](noncompliant02.py):* | ||
|
|
||
| ```python | ||
| """Non-compliant Code Example""" | ||
|
|
||
dwiley258 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| # SPDX-FileCopyrightText: OpenSSF project contributors | ||
| # SPDX-License-Identifier: MIT | ||
| """ Non-compliant Code Example """ | ||
| import os | ||
| from tempfile import mkstemp | ||
|
|
||
| fd, path = mkstemp() | ||
| with os.fdopen(fd, 'w') as f: | ||
| f.write('TEST\n') | ||
|
|
||
| print(path) | ||
| ``` | ||
|
|
||
| The non-compliant02.py does not remove the file after use and leaves clean up to the user or the operating system. | ||
dwiley258 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| ## Compliant Solution | ||
|
|
||
| In compliant01.py we use the tempFile module to generate our temporary file. When not passing in delete=false the default behaviour is the fie will be deleted after the corresponding file-like objects are closed. | ||
dwiley258 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| Thanks to the use of the "with" statement we ensure that the file is closed after the file is written, even if an error is to occur. | ||
dwiley258 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| *[compliant01.py](compliant01.py):* | ||
|
|
||
| ```python | ||
| """Compliant Code Example""" | ||
|
|
||
| # SPDX-FileCopyrightText: OpenSSF project contributors | ||
| # SPDX-License-Identifier: MIT | ||
| """ Compliant Code Example """ | ||
| import tempfile | ||
|
|
||
| with tempfile.NamedTemporaryFile() as temp_file: | ||
| temp_file.write(b'This temporary file will be deleted.') | ||
| temp_file_path = temp_file.name | ||
|
|
||
| with open(temp_file_path, 'rb') as temp_file: | ||
| print(temp_file.read()) | ||
| ``` | ||
|
|
||
| In the first with block of compliant01.py, a temporary file is created, which will be automatically deleted once the block is exited. It is expected that the second with block will return a FileNotFoundError as the file will have been successfully deleted. | ||
|
|
||
| ## Automated Detection | ||
|
|
||
| |Tool|Version|Checker|Description| | ||
| |:---|:---|:---|:---| | ||
| |Pylint|3.3.7 on python 3.10.4|[W1514:unspecified-encoding](https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/unspecified-encoding.html)|Using open without explicitly specifying an encoding| | ||
|
|
||
| ## Related Guidelines | ||
|
|
||
| ||| | ||
| |:---|:---| | ||
| |[MITRE CWE](http://cwe.mitre.org/)|Pillar: [CWE-664: Improper Control of a Resource Through its Lifetime (mitre.org)](https://cwe.mitre.org/data/definitions/664.html)| | ||
| |[MITRE CWE](http://cwe.mitre.org/)|Base: [CWE-459: Incomplete Cleanup](https://cwe.mitre.org/data/definitions/459.html)| | ||
| |[SEI CERT Coding Standard for Java](https://wiki.sei.cmu.edu/confluence/display/java/SEI+CERT+Oracle+Coding+Standard+for+Java)|[FIO03-J. Remove temporary files before termination](https://wiki.sei.cmu.edu/confluence/display/java/FIO03-J.+Remove+temporary+files+before+termination)| | ||
11 changes: 11 additions & 0 deletions
11
docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-459/compliant01.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,11 @@ | ||
| # SPDX-FileCopyrightText: OpenSSF project contributors | ||
| # SPDX-License-Identifier: MIT | ||
| """ Compliant Code Example """ | ||
| import tempfile | ||
|
|
||
| with tempfile.NamedTemporaryFile() as temp_file: | ||
| temp_file.write(b'This temporary file will be deleted.') | ||
| temp_file_path = temp_file.name | ||
|
|
||
| with open(temp_file_path, 'rb') as temp_file: | ||
| print(temp_file.read()) | ||
dwiley258 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
6 changes: 6 additions & 0 deletions
6
docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-459/noncompliant01.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,6 @@ | ||
| # SPDX-FileCopyrightText: OpenSSF project contributors | ||
| # SPDX-License-Identifier: MIT | ||
| """ Non-compliant Code Example """ | ||
| f = open("tempfile.txt", "w") | ||
| f.write("temporary file created!") | ||
| f.close() | ||
dwiley258 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
11 changes: 11 additions & 0 deletions
11
docs/Secure-Coding-Guide-for-Python/CWE-664/CWE-459/noncompliant02.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,11 @@ | ||
| # SPDX-FileCopyrightText: OpenSSF project contributors | ||
| # SPDX-License-Identifier: MIT | ||
| """ Non-compliant Code Example """ | ||
| import os | ||
| from tempfile import mkstemp | ||
|
|
||
| fd, path = mkstemp() | ||
| with os.fdopen(fd, 'w') as f: | ||
| f.write('TEST\n') | ||
|
|
||
| print(path) | ||
dwiley258 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.
Uh oh!
There was an error while loading. Please reload this page.