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

Invalid CVSS v2 environmental score computation #33

Open
pandatix opened this issue Feb 3, 2023 · 12 comments
Open

Invalid CVSS v2 environmental score computation #33

pandatix opened this issue Feb 3, 2023 · 12 comments
Labels
bug Something isn't working

Comments

@pandatix
Copy link

pandatix commented Feb 3, 2023

During differential fuzzing with github.com/pandatix/go-cvss I discovered that your implementation does not properly computes CVSS v2 environmental scores (as for #18).

For instance, the vector AV:A/AC:L/Au:N/C:C/I:C/A:C/CDP:H/TD:H/CR:L/IR:ND/AR:ND have an environmental score of 9.0, according to the NVD CVSS v2 calculator. Nevertheless, the following Go code illustrates this issue i.e. invalid scores.

package main

import (
	"fmt"
	"log"

	"github.com/goark/go-cvss/v2/metric"
)

func main() {
	raw := "AV:A/AC:L/Au:N/C:C/I:C/A:C/CDP:H/TD:H/CR:L/IR:ND/AR:ND"
	vec, err := metric.NewEnvironmental().Decode(raw)
	if err != nil {
		log.Fatal(err)
	}

	b, t, e := vec.Base.Score(), vec.Temporal.Score(), vec.Score()
	fmt.Printf("Scores: %.1f;%.1f;%.1f\n", b, t, e)
}

produces ->

Scores: 8.3;8.3;9.1
@spiegel-im-spiegel
Copy link
Member

Release v1.6.4.

@pandatix
Copy link
Author

pandatix commented Feb 4, 2023

It was not fixed properly, as the vector AV:L/AC:M/Au:S/C:N/I:N/A:P/CDP:N/TD:ND/CR:M/IR:ND/AR:ND now produces an environmental score of 1.6 despite it effectively being 1.5.

spiegel-im-spiegel added a commit that referenced this issue Feb 4, 2023
Adjusted calculation error of CVSSv2 Base score (issue #33)
@spiegel-im-spiegel
Copy link
Member

spiegel-im-spiegel commented Feb 4, 2023

Hmmm... Released v1.6.5. Is it OK?

@pandatix
Copy link
Author

pandatix commented Feb 4, 2023

Now, the vector AV:A/AC:M/Au:S/C:C/I:C/A:C/CDP:N/TD:N/CR:M/IR:ND/AR:L returns an environmental score of 7.1 but should be 0.0.

spiegel-im-spiegel added a commit that referenced this issue Feb 4, 2023
Fixed bug of Calculation of CVSSv2 Environmental score (issue #33)
@spiegel-im-spiegel
Copy link
Member

Sorry! Released v1.6.6.

@spiegel-im-spiegel
Copy link
Member

Thank you for many advices.

@pandatix
Copy link
Author

Sorry, I still have some issues to raise about this, but I first need to fix the NVD due to the same issue :')

@pandatix
Copy link
Author

pandatix commented Apr 3, 2023

Hi, sorry for the looong wait, seems like they won't fix it... So here is the issue !

Let's take again the CVSS v2 vector "AV:A/AC:L/Au:N/C:C/I:C/A:C/CDP:H/TD:H/CR:L/IR:ND/AR:ND" with Base and Environmental groups defined.
If we strictly apply the specified formulas section 3.2.3 for the adjusted impact, we end up with 9.60372468.
I didn't deeply checked where the issue occurs in your implementation, but for instance in the NVD implementation it occurs when computing the adjusted temporal. With formulas you have round_to_1_decimal(((0.6*9.60372468)+(0.4*6.4579328)-1.5)*1.176) = round_to_1_decimal(8.050199723328) = 8.1, when your implementation returns 8.0.
Maybe, due to this rounding edge case, you return an environmental score of 9.0 rather than 9.1 (would be worth to check this first).

@spiegel-im-spiegel
Copy link
Member

🤔 ...

@pandatix
Copy link
Author

Hey, any news on fixing it ?
It may be nice to get it before we release CVSS v4.0 thus your implementation could be used without this rounding issue :)

@spiegel-im-spiegel
Copy link
Member

sorry.
I'm too busy with my day job to fix go-cvss.
There are no specific plans for CVSSv4 either.

@pandatix
Copy link
Author

Oh, I'm sorry to hear that. If you think this is appropriate maybe archive the project to let it available while read-only. Hope you do well

another-rex pushed a commit to google/osv-scanner that referenced this issue Nov 30, 2023
…h the specifications (#651)

## Why this PR

[CVSS v4.0](https://www.first.org/cvss/v4-0/) has been released lately,
and the OSV will most probably add its support (the first CVSS v4.0
vector known to the FIRST.ORG SIG CVSS has been published [by Palo Alto
Networks for the
CVE-2023-3282](https://security.paloaltonetworks.com/CVE-2023-3282)).

As a FIRST.ORG SIG CVSS member and [Go CVSS
implementation](https://github.com/pandatix/go-cvss) maintainer, I'm
looking forward to improve its adoption and understanding in the
Open-Source Ecosystem.
Moreover, there exist issues with the currently used CVSS
implementation, such as [invalid scoring
computation](goark/go-cvss#33), and [CVSS v4.0
is currently not planned for
support](goark/go-cvss#37 (comment)).

## What it brings

With the current PR, I provide multiple direct improvements:
- proper CVSS v2.0 scoring computation (only affect the environmental
score computation, but has been an unresolved issue for months)
- add support of CVSS v4.0 in the OSV schema
- performance improvements according to
[benchmarks](https://github.com/pandatix/go-cvss#comparison)

Given ossf/osv-schema#166 the CVSS v4.0 key will most likely be
`CVSS_V4` to align with the previous CVSS versions support.

## Is it breaking ?

For the code, no, but for the Go version, yes 🎉
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants