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

Improper Input Validation in CVSS v2 parsing #1

Open
pandatix opened this issue Jan 30, 2023 · 3 comments
Open

Improper Input Validation in CVSS v2 parsing #1

pandatix opened this issue Jan 30, 2023 · 3 comments
Assignees

Comments

@pandatix
Copy link

During differential fuzzing with github.com/pandatix/go-cvss, I discovered that your implementation does not properly validate inputs.
This could be categorized as CWE-20.

For instance, the input AV:A/AC:L/Au:N/C:C/I:C/A:C/CDP:H/TD:H/CR:H is an invalid CVSS v2 vector as environmental metrics are not defined (IR and AR) where they should all be. This could be verified with the first.org specification Table 13 that shows they are mandatory when the environmental group is specified in the vector.

In order to be compliant, you must check all metrics from a group are specified.

The following Go code illustrates this issue.

package main

import (
	"fmt"

	"go.zenithar.org/mitre/pkg/services/cvss/v2/vector"
)

func main() {
	raw := "AV:A/AC:L/Au:N/C:C/I:C/A:C/CDP:H/TD:H/CR:H"
	vec, err := vector.FromString(raw)

	fmt.Printf("vec: %v\n", vec)
	fmt.Printf("err: %v\n", err)
}

produces ->

vec: base_metrics:<access_vector:ACCESS_VECTOR_ADJACENT_NETWORK access_complexity:ACCESS_COMPLEXITY_LOW authentication:AUTHENTICATION_NONE confidentiality_impact:CONFIDENTIALITY_IMPACT_COMPLETE integrity_impact:INTEGRITY_IMPACT_COMPLETE availability_impact:AVAILABILITY_IMPACT_COMPLETE > environmental_metrics:<collateral_damage_potential:COLLATERAL_DAMAGE_POTENTIAL_HIGH target_distribution:TARGET_DISTRIBUTION_HIGH confidentiality_requirement:SECURITY_REQUIREMENT_HIGH > 
err: <nil>
@Zenithar Zenithar self-assigned this Mar 2, 2023
@pandatix
Copy link
Author

pandatix commented Mar 2, 2023

You may need to create a tag so dependencies could update without having to target master 😉

@pandatix
Copy link
Author

pandatix commented Mar 2, 2023

Still during differential fuzzing, I discovered that your implementation suffers from another validation issue. It needs this issue to be re-opened. For input vector AV:A/AC:H/Au:N/C:C/I:C/A:C/Au:N, your implementation does not return any error despite metrics are not ordered properly.

This could be verified with the first.org specification Table 13 that shows they are "in a predetermined order" (Section 2.4, 2nd sentence).

In order to be compliant, you must validate metrics are in the predetermined order.

@Zenithar Zenithar reopened this Mar 9, 2023
@Zenithar
Copy link
Collaborator

@pandatix Thank for all your work. This library is very old, and reminds me good old projects.

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

No branches or pull requests

2 participants