Skip to content
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-755 and CWE-532 #647

Merged
merged 20 commits into from
Oct 24, 2024
Merged

CWE-755 and CWE-532 #647

merged 20 commits into from
Oct 24, 2024

Conversation

BartyBoi1128
Copy link
Contributor

@BartyBoi1128 BartyBoi1128 commented Oct 9, 2024

Adding documentation and code for CWE-755 and CWE-532, pyDoc2GitHub as per #531

Signed-off-by: ebakrra <[email protected]>
@myteron myteron self-requested a review October 9, 2024 10:38
Copy link
Contributor

@myteron myteron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the main readme.md is missing the entries for CWE-532 and CWE-755
https://github.com/ossf/wg-best-practices-os-developers/blob/main/docs/Secure-Coding-Guide-for-Python/readme.md

Add following SPDX headers to at least the top of the .py code examples. No need to quote SPDS in the README.md's

# SPDX-FileCopyrightText: OpenSSF project contributors
# SPDX-License-Identifier: MIT


## Example Solution

The `example01.py` solution uses a custom filter in Python's logging module to automatically mask sensitive data in the logs. While this is a good solution for central handling in large software project it does require that all modules use the same string such as password=, variation's such as pass= or pass: won't work and continue to print the plain text password.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backtick inline code examples or names:
password=
pass=


For security purposes, sensitive information should never be printed to the console in log messages (for instance, a passenger's age). In Python's logging module, there are five logging levels:

* DEBUG
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backtick any inline code examples or names in below bullet list and following sentence DEBUG or INFO


|Tool|Version|Checker|Description|
|:----|:----|:----|:----|
|[Pylint](https://pylint.pycqa.org/)|2023.10.1|[W1203:logging-fstring-interpolation](W1203:logging-fstring-interpolation)|Use lazy % formatting in logging functions|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing the link in the (0) its not W1203:logging-fstring-interpolation


|Tool|Version|Checker|Description|
|:----|:----|:----|:----|
|[Pylint](https://pylint.pycqa.org/)|2023.10.1|[W1203:logging-fstring-interpolation](W1203:logging-fstring-interpolation)|Use lazy % formatting in logging functions|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

References is also missing the 4 links in the introduction.

*[compliant01.py](compliant.py):*

```py
""" Non-compliant Code Example """
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Says """Non compliant .... expecting """ Compliant


The file opening and reading should be surrounded by the `try/except` block. This way, we can catch the generic `OSError` and handle the error differently depending on its cause (such as the file not existing or it being a directory instead).

*[compliant01.py](compliant.py):*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the link in the brackets is missing 01, should be (compliant01.py)

read_file(f"{uuid.uuid4()}.txt")
```

## Risk Assessment
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove Risk Assessment section

Added missing links and backticks as per comments

Signed-off-by: BartyBoi1128 <[email protected]>
Updated compliant01 mistakes, and removed Risk Assessment section

Signed-off-by: BartyBoi1128 <[email protected]>
Updated references

Signed-off-by: BartyBoi1128 <[email protected]>
Updated references again.

Signed-off-by: BartyBoi1128 <[email protected]>
@myteron myteron removed the request for review from gkunz October 24, 2024 10:56
@myteron
Copy link
Contributor

myteron commented Oct 24, 2024

got approval from 2 reviewers, merging in

@myteron myteron merged commit 67489e1 into ossf:main Oct 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants