Still some floating point error

[Xsze]

Hello,

Following the last update that should fixes floating point errors, I noticed that there were still some rounding errors due to floating point residual in the python code.

Used vector : CVSS:4.0/AV:L/AC:H/AT:N/PR:N/UI:N/VC:L/VI:H/VA:L/SC:N/SI:N/SA:H/S:N/AU:N/R:I/V:C/RE:M/U:Green/MAV:A/MAC:L/MAT:N/MPR:N/MUI:N/MVC:H/MVI:N/MVA:H/MSC:H/MSI:H/MSA:H/CR:H/IR:M/AR:H/E:A

Final result : 8.5 (on python and the online Java calculator)

Should have been 8.6 as the final result is ,normaly , 8.55 rounded up to 8.6
Or if we look inside vars we can see the final result before rounding is 8.549999999999999 cause of floating point again so it round it to 8.5.

It seems to come from the available distance step :

8.6-7.2 = 1.3999999999999995
8.6-7.4=1.1999999999999993

I didn't analyse the Java online calculator but as it return the same result it should be impacted as well.

[Xsze]

Here with another vector : CVSS:4.0/AV:A/AC:H/AT:P/PR:L/UI:N/VC:H/VI:H/VA:H/SC:L/SI:H/SA:N/S:N/AU:Y/R:A/V:D/RE:H/U:Red/MAV:L/MAC:L/MAT:P/MPR:H/MUI:P/MVC:N/MVI:H/MVA:L/MSC:N/MSI:L/MSA:L/CR:L/IR:L/AR:L/E:P

final result : 0.5 instead of 0.6

Final result before rounding :
0.5499999999999999 instead of 0.55

Strangely it seems to no touch every available_distance result....
vector: CVSS:4.0/AV:N/AC:L/AT:P/PR:H/UI:A/VC:L/VI:N/VA:L/SC:H/SI:H/SA:H

Here, only the "available_distance_eq5" 5.8-2.6 is a float result...
but the final result is still false : 4.9 instead of 5.0 cause of rounding 4.949999999999999 instead of 4.95

In general rule, every result that shoud be x.x5 before rounding is false cause they are all x.x49999999999999 and rounded down instead of up.

For info, in the test files given i get
522 wrong final results on file vectors_base4
1044 wrong final results on file vectors_modified4
367 wrong final results on file vectors_random4
0 wrong final result on file vectors_security4
0 wrong final result on file vectors_supplemental4
0 wrong final result on file vectors_threat4
So i can give you more vector with wrong result if needed.

[skontar]

@Xsze thanks for reporting!

It is unfortunate, that CVSS SIG decided to use floating point arithmetic for score computations instead of fixed point arithmetic causing these issues.

Currently, the goal of this project is to match results from official implementation at https://github.com/RedHatProductSecurity/cvss-v4-calculator and https://www.first.org/cvss/calculator/4.0 (these should match). The CVSSv4.0 specification does not have any recommendation for handling of rounding the same way the v3.1 has (see Appendix A - Floating Point Rounding).

The best way to get this improved is to either contact CVSS SIG directly https://support.first.org/servicedesk/customer/portal/1/create/4 or opening an issue at https://github.com/RedHatProductSecurity/cvss-v4-calculator where some of the members are active (and I can bring that to their attention).

[skontar]

@Xsze I see that you did a lot of investigation on this issue. Would you be able to provide test vector files with your expected values?

[Xsze]

Of course, here they are.
vectors_base4_divergence.txt
vectors_random4_divergence.txt
vectors_modified4_divergence.txt

It's based on the test files you provided.

[skontar]

Awesome. I will try some workarounds for float rounding when I find some spare time.

[Xsze]

If the use of floating point is mandatory, i think something like this could eliminate the problem without to mess with all the code:

if "9999999999999" in str(value):
     value = round_away_from_zero(value*10)/10

self.base_score = round_away_from_zero(value)

Just before the final rounding, if the value before rounding contains 9999999999999 a pre rounding to 2 decimal is done to get the variable nearer to what it should be and then you do the final rounding like now.

CVSS:4.0/AV:N/AC:L/AT:P/PR:H/UI:A/VC:N/VI:L/VA:H/SC:N/SI:L/SA:N
here, the actual method give 5.6 for this vector but with the pre rounding you get the correct 5.7 final answear.

I've posted the Java version in the issue 66

I've tested it on the 30000 first line of vector_base4 and It need some more test to see if it impact anything else but for now everything seems good. No more false results.

[skontar]

I am more thinking about pre-rounding to more decimal places and then rounding to one. Checking for string of nines is really hacky. Floating points are not mandatory, but rewriting whole CVSS v4.0 algorithm to Decimals would be painful and then we could end in a situation when it could be hard to match in other implementations.

I am thinking something like (not tested yet):

def final_rounding(x):
    """
    Round to one decimal place. Use Decimal because Python float rounding defaults to
    "round half to even". We actually want "round half away from zero" aka "round half up" for
    positive numbers.

    Make sure that values like the following are correctly rounded despite flouting point
    inaccuracies:

    8.6 - 7.15 = 1.4499999999999993 (float) ➔ 1.5
    """
    pre_rounded = D(x).quantize(D("0.001"), rounding=ROUND_HALF_UP)
    return float(D(pre_rounded).quantize(D("0.1"), rounding=ROUND_HALF_UP))

[skontar]

@Xsze Looking at the files you posted, would you be able to easily generate test vectors like they are in the project, but with "fixed" values (so they are drop in replacements)?

[Xsze]

It's hacky but it works 👍 Tested it on 75% of base 4 and Random4 files and 50% of the modified4 and no errors show up.

I have already tested something as you propose but with a pre rounding to 0.01 first on every result, it corrected the said wrong results but it introduced a lot more new one, that's why i took the ''if 999999'' route to limit the scope of the pre rounding to just the important one. So maybe with a wider pre rounding to 0.001 like you propose or even 0.0001 to reduce the risk to interfer with the good result it might work.

Anyway, here are the file requested.
vectors_random4_NewOutput.txt
vectors_base4_NewOutput.txt
vectors_modified4_NewOutput_Part1.txt
vectors_modified4_NewOutput_Part2.txt

I had to cut the modified4 in half because we are limited to 25Mo in upload in the comment

[skontar]

@Xsze please check #65 . Looks like it is working as expected.

[Xsze]

Just tested with Base and Random and all seems good to me.

Now, it only remains to apply the correction to the online calculator.

Edit: Also stested the modified4 file and no errors show up eiter.

[skontar]

I have another colleague to check that, test and fix, next week. Until then, I will keep the PR blocked, so we can do it at once.

[skontar]

@Xsze after discussion with CVSS SIG members we are discussing a different, simpler fix. Would you mind verifying the following implementations?
RedHatProductSecurity/cvss-v4-calculator#67
#65

[Xsze]

Seems still good to me.

[skontar]

@Xsze thank you very much for your contributions and patience.