RedHatProductSecurity / cvss

CVSS2/3/4 library with interactive calculator for Python 2 and Python 3
GNU Lesser General Public License v3.0
82 stars 30 forks source link

Add support for CVSSv4 #45

Closed jobiewinserapck closed 10 months ago

jobiewinserapck commented 10 months ago

-Added class CVSS4 -Based the CVSS4 class on the js implementation found here: https://github.com/RedHatProductSecurity/cvss-v4-calculator -Added new tests for CVSS4 -Added new constants and exceptions for CVSS4 -Added json schema for CVSS4

skontar commented 10 months ago

This looks really good. Thank you!

skontar commented 10 months ago

@mprpic can you please check CI? It seems to fail for some reason.

skontar commented 10 months ago

@jobiewinserapck I presume you used the js implementation to generate test vectors?

jobiewinserapck commented 10 months ago

Yes, I generated the vectors using the js implementation. I could Maybe get some screenshots tomorrow of how I did so. If thats helpful.

The only test failures I encountered after writing this was due to python3's rounding not being "away from zero", causing scores such as 7.25 to round towards 7.2 rather than 7.3 as expected (hence the round_away_from_zero func)

skontar commented 10 months ago

No need for screenshots, just curious. Rounding was always "fun" previously. I am glad you resolved it easily.

skontar commented 10 months ago

@jobiewinserapck can you rebase with master? I think that should re-run the CI with latest setup.

jobiewinserapck commented 10 months ago

Sure thing I'll do that ASAP tomorrow morning

skontar commented 10 months ago

@mprpic can you please review?

jobiewinserapck commented 10 months ago

I'ved added those improvements, thanks for that @skontar!

marco-silva0000 commented 10 months ago

just commenting for support, gogogo

jobiewinserapck commented 10 months ago

Just commenting to say I've reformatted cvss/cvss4.py, hopefully the pipeline will pass on next run

jobiewinserapck commented 10 months ago

I've pushed up a commit addressing the issues raised by @mprpic.

I also added some additional tests for malformed cvss strings

skontar commented 10 months ago

@mprpic please re-review.

skontar commented 8 months ago

@jobiewinserapck there were recent changes to rounding in official Javascript implementation. Would you be so kind and double-check everything is still shiny and both implementations match? I think that the Python one will be more correct as it uses Decimal.

AdrianVollmer commented 8 months ago

I don't think round_away_from_zero is correct:

$ python << EOF
from decimal import ROUND_HALF_UP
from decimal import Decimal as D
def round_away_from_zero(x, dp=1):
    return float(D(x).quantize(D("0." + "0" * dp), rounding=ROUND_HALF_UP))

for i in range(6):
    print(round_away_from_zero(i + 0.05))
EOF
0.1
1.1
2.0
3.0
4.0
5.0

It's inconsistent.

I would have said we should just do round(x*10)/10 like the JavaScript implementation, but Math.round(0.5) is 1 in JS and round(0.5) is 0 in Python3 and 1 in Python2. Python3 follows IEEE 754.

The specification document is not clear on which rounding convention should be used:

This score is rounded to one decimal place.
skontar commented 8 months ago

Correct .quantize call should be able to resolve that difference. I do not recall what we did for v3.1 but we were able to get exactly the same results between Python and Javascript.

skontar commented 8 months ago

@AdrianVollmer if the input is Decimal then round_away_from_zero works correctly I think.

>>> float((D(2) + D(5) / D(100)).quantize(D("0.0"), rounding=ROUND_HALF_UP))
2.1
AdrianVollmer commented 8 months ago

Okay, but this function is only used once here and is passed a float:

https://github.com/RedHatProductSecurity/cvss/blob/c5dd7e2f0bcda1b32d509cce59a6db7c94bbec64/cvss/cvss4.py#L558

AdrianVollmer commented 8 months ago

I think this function will suit this particular use case:

def round_away_from_zero(value):
    # Only works correctly for positive values
    return int(value * 10 + 0.5) / 10.0

Since we only need to round values between 0.0 and 10.0, it should be fine.

$ python << EOF
for i in range(10):
 print(int((i+0.05)*10 + 0.5)/10)
EOF
0.1
1.1
2.1
3.1
4.1
5.1
6.1
7.1
8.1
9.1