RedHatProductSecurity / cvss

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

Minor CVSSv2 Score difference with NVD. #10

Closed UberMeatShield closed 7 years ago

UberMeatShield commented 7 years ago

The library is great! While using it one of our people noticed a tiny scoring difference between the library and NVD and I am not actually certain which is correct.

https://nvd.nist.gov/cvss.cfm?version=2&vector=(AV:N/AC:L/Au:S/C:C/I:C/A:C/E:U/RL:U/RC:C) Returns 7.7

from cvss import CVSS2, CVSS3 vector_1 = 'AV:N/AC:L/Au:S/C:C/I:C/A:C/E:U/RL:U/RC:C' CVSS2(vector_1).scores()

(9.0, 7.6, None)

https://nvd.nist.gov/cvss.cfm?version=2&vector=(AV:N/AC:L/Au:S/C:C/I:C/A:C/E:F/RL:U/RC:C) Returns 8.7

from cvss import CVSS2, CVSS3 vector_2 = 'AV:N/AC:L/Au:S/C:C/I:C/A:C/E:F/RL:U/RC:C' CVSS2(vector_2).scores()

(9.0, 8.6, None)

skontar commented 7 years ago

Hi. I will look into it as time permits. I am guessing it is going to be a rounding problem. The problem you see seems to be in temporal score, correct?

UberMeatShield commented 7 years ago

Yes, probably just a simple round but when I quickly looked through the code I didn't see anything obvious.

On Mon, Nov 7, 2016 at 11:47 AM, Stanislav Kontár notifications@github.com wrote:

Hi. I will look into it as time permits. I am guessing it is going to be a rounding problem. The problem you see seems to be in temporal score, correct?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/skontar/cvss/issues/10#issuecomment-258890330, or mute the thread https://github.com/notifications/unsubscribe-auth/ACZ9hs-QxH_QkEt3y5dZYSZ8iYGrwAtDks5q71YhgaJpZM4KrVzW .

skontar commented 7 years ago

Can you check your second example in the calculator? It seems to me to return 8.5 as temporal score.

UberMeatShield commented 7 years ago

Apologies, you are correct. The second example was returning 8.5 as the temporal, copy paste error probably. The calculator does return 8.6 for me though so still slightly different (at least using the released version from pypi).

skontar commented 7 years ago

So I know where are the problems:

  1. I use exact values of fractions, e.g. Python Decimal type, Javascript uses 64-bit float, which is slightly off.
  2. The default Python rounding is ROUND_HALF_EVEN. Javascript probably uses ROUND_HALF_UP.

Specification does not specify what rounding is correct, or the expected data types are. Both examples end up with x.x5 values, triggering one or both conditions and resulting in different result.

skontar commented 7 years ago

I am thinking that ROUND_HALF_UP is probably expected by most people, so I may change that. However I am not convinced that using float for computations is better than Decimal. I will think about that more and do some testing.

skontar commented 7 years ago

We have discussed the issue in Red Hat Product Security team.

I have planned to change the rounding algorithm to more usual ROUND_HALF_UP which is probably expected by anyone and is something we learned at school.

I also plan to leave Decimal data type as means of computation, as these are exact and do not exhibit problems with floating point arithmetic. In my opinion the following two examples give weird results when computed as float:

Exact: 9 * 0.85 = 7.65 ≈ 7.7 Float: 9 * 0.85 = 7.6499999999999995 ≈ 7.6

Exact: 9 * 0.95 = 8.55 ≈ 8.6 Float: 9 * 0.95 = 8.549999999999999 ≈ 8.5

Reverse engineering NVD calculator has shown that they use weird ways how to handle float problem which does not work well for some of the situations (so they correctly compute 7.7 but incorrectly 8.5).

I should be able to fix the code and make sure tests are correct some time next week.

I hope that helps.

UberMeatShield commented 7 years ago

That is fantastic! Thank you!

On Sat, Nov 12, 2016 at 3:53 PM, Stanislav Kontár notifications@github.com wrote:

We have discussed the issue in Red Hat Product Security team.

I have planned to change the rounding algorithm to more usual ROUND_HALF_UP which is probably expected by anyone and is something we learned at school.

I also plan to leave Decimal data type as means of computation, as these are exact and do not exhibit problems with floating point arithmetic. In my opinion the following two examples give weird results when computed as float:

Exact: 9 * 0.85 = 7.65 ≈ 7.7 Float: 9 * 0.85 = 7.6499999999999995 ≈ 7.6

Exact: 9 * 0.95 = 8.55 ≈ 8.6 Float: 9 * 0.95 = 8.549999999999999 ≈ 8.5

Reverse engineering NVD calculator has shown that they use weird ways how to handle float problem which does not work well for some of the situations (so they correctly compute 7.7 but incorrectly 8.5).

I should be able to fix the code and make sure tests are correct some time next week.

I hope that helps.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/skontar/cvss/issues/10#issuecomment-260147730, or mute the thread https://github.com/notifications/unsubscribe-auth/ACZ9hijKX3AojIunxXd-hUt30XuUy_koks5q9idagaJpZM4KrVzW .

skontar commented 7 years ago

Fixed with f0bbd4e02dbebe197cc01e1de92f07f1fa900a21 .

UberMeatShield commented 7 years ago

No rush but any chance of this making it into a 1.6 release? I would love to point my CI at something not master :)

skontar commented 7 years ago

Sure thing.

skontar commented 7 years ago

Done.