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

pyDoc2GitHub Adding documentation for CWE-197 01 as part of #531 to GitHub #649

Merged
merged 3 commits into from
Oct 17, 2024

Conversation

myteron
Copy link
Contributor

@myteron myteron commented Oct 9, 2024

Signed-off-by: Helge Wehder [email protected]
part of #531

Copy link
Contributor

@s19110 s19110 left a comment

Choose a reason for hiding this comment

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

I left a few comments, mostly regarding cosmetics.


```

The build in `round()` does not allow to specify the type of rounding in use [[python round( ) 2024]](https://docs.python.org/3/library/functions.html#round). In Python 3 the `round()` function uses "bankers' rounding" (rounds to the nearest even number in case of ties). This is different to Python 2 which always rounds away from zero. Rounding provided by the decimal module allows a choice between 8 rounding modes [python decimal 2024](https://docs.python.org/3/library/decimal.html#rounding-modes). Rounding in mathematics and science is not discussed here as it requires a deeper knowledge of computer floating-point arithmetic's.
Copy link
Contributor

Choose a reason for hiding this comment

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

cosmetics:


## Compliant Solution (float to int)

Using the `Decimal` module allows more control over rounding by choosing one of the 8 rounding modes in the decimal module.
Copy link
Contributor

Choose a reason for hiding this comment

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

Haven't caught that in the original review, but this sentence is a bit confusing with the "Decimal" and the "decimal" modules. Maybe we could rewrite it to something like that:

Using the Decimal class from the decimal module allows more control over rounding by choosing one of the 8 rounding modes.

print(Decimal("1.5").quantize(Decimal("1"), rounding=ROUND_HALF_DOWN)) # prints 1
```

The `.quantize(Decimal("1")`, determines the precision to be integer and `rounding=ROUND_HALF_UP` determines the type of rounding applied. Specifying numbers as strings avoids issues such as floating-point representations in binary.
Copy link
Contributor

Choose a reason for hiding this comment

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

cosmetics: integer

```py
""" Code Example """
# SPDX-FileCopyrightText: OpenSSF project contributors
# SPDX-License-Identifier: MIT
Copy link
Contributor

Choose a reason for hiding this comment

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

cosmetics: license comments in the Markdown code

|[MITRE CWE](http://cwe.mitre.org/)|Class [CWE-197, Numeric Truncation Error](https://cwe.mitre.org/data/definitions/197.html)|
|[SEI CERT C Coding Standard](https://wiki.sei.cmu.edu/confluence/display/c/SEI+CERT+C+Coding+Standard)|[INT31-C. Ensure that integer conversions do not result in lost or misinterpreted data](https://wiki.sei.cmu.edu/confluence/display/c/INT31-C.+Ensure+that+integer+conversions+do+not+result+in+lost+or+misinterpreted+data)|
|[SEI CERT C Coding Standard](https://wiki.sei.cmu.edu/confluence/display/c/SEI+CERT+C+Coding+Standard)|[FLP34-C. Ensure that floating-point conversions are within range of the new type](https://wiki.sei.cmu.edu/confluence/display/c/FLP34-C.+Ensure+that+floating-point+conversions+are+within+range+of+the+new+type)|
|[ISO/IEC TR 24772:2019]|Programming languages — Guidance to avoiding vulnerabilities in programming languages, available from [https://www.iso.org/standard/71091.html](https://www.iso.org/standard/71091.html)|
Copy link
Contributor

Choose a reason for hiding this comment

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

The Confluence page links to web archive: ISO/IEC TR 24772:2019

|||
|:---|:---|
|[python round( ) 2024]|python round( ), available from: [https://docs.python.org/3/library/functions.html#round](https://docs.python.org/3/library/functions.html#round), [Last accessed June 2024] |
|[python decimal 2024]|Python decimal module, available from: [https://docs.python.org/3/library/decimal.html#rounding-modes](https://docs.python.org/3/library/decimal.html#rounding-modes)|
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing hyperlinks:
-- [python round( ) 2024]
-- [python decimal 2024]

Adding link/reference to 8 rounding modes

Signed-off-by: myteron <[email protected]>
Copy link
Contributor

@BartyBoi1128 BartyBoi1128 left a comment

Choose a reason for hiding this comment

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

Approving this change!

@myteron myteron merged commit e247db1 into ossf:main Oct 17, 2024
2 checks passed
@myteron myteron deleted the pyDoc2GitHub_CWE-197-01 branch October 17, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants