Skip to content

Improve GitHub vulnerability adding affected version#9457

Closed
Demaz93 wants to merge 1 commit intoDefectDojo:devfrom
Demaz93:improve_github_parser
Closed

Improve GitHub vulnerability adding affected version#9457
Demaz93 wants to merge 1 commit intoDefectDojo:devfrom
Demaz93:improve_github_parser

Conversation

@Demaz93
Copy link
Copy Markdown
Contributor

@Demaz93 Demaz93 commented Feb 1, 2024

Description

Today component version field in GitHub Vulnerability import is not managed.
This PR will add the vulnerable range for the specific component that could be useful for developers.

Test results

I tested the import from GitHub to defectdojo.

Documentation

Examples inside the documentation already import the vulnerable range but the parser ignored it.

Extra information

Please clear everything below when submitting your pull request, it's here purely for your information.

@github-actions github-actions bot added the parser label Feb 1, 2024
@dryrunsecurity
Copy link
Copy Markdown

dryrunsecurity bot commented Feb 1, 2024

Contextual Security Analysis

As DryRun Security performs checks, we’ll summarize them here. You can always dive into the detailed results in the section below for checks.

Status DryRun Security Check
Sensitive Functions Analyzer
Configured Sensitive Files Check
Sensitive Files Analyzer

Chat with your AI-powered Security Buddy by typing @dryrunsecurity followed by your question into a comment.
Example: @dryrunsecurity What are common security issues with web application cookies?

Install and configure more repositories at DryRun Security

@Demaz93 Demaz93 marked this pull request as ready for review February 1, 2024 14:32
@kiblik
Copy link
Copy Markdown
Contributor

kiblik commented Feb 1, 2024

Can you implement unittest as it is recommended in https://documentation.defectdojo.com/contributing/how-to-write-a-parser/#unit-tests please?

Copy link
Copy Markdown
Contributor

@coheigea coheigea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the PR #9462 is a better solution (and adds a test). This PR adds the vulnerable version range, however the component version is supposed to be the actual version that is vulnerable. #9462 extracts the actual vulnerable version from the GitHub data.

@Demaz93
Copy link
Copy Markdown
Contributor Author

Demaz93 commented Feb 2, 2024

Theoretically, you are right, vulnerableRequirements fits better, but in case you have multiple occurrences in the same file you probably lose something to fix because you are looking for the exact provided version for example:
Vulnerability related to library XYZ with vulnerable version < 1.2.3
in my code, I use version 0.8.2 and version 1.1.1 (because related to some dependencies of different libraries)

In this case, GitHub provides only one vulnerability and inside Defectdojo I'll have one finding with only 1.1.1 or 0.8.2 vulnerable version, which is false because I have two occurrences and I have to fix both.
Instead in this PR, you have a range so you have to check all the occurrences for the same library.

What do you think @coheigea about this edge-case scenario?

@coheigea
Copy link
Copy Markdown
Contributor

coheigea commented Feb 2, 2024

Hi @Demaz93 ,
Can you attach a sample scan result showing multiple occurences coming from the same github vulnerability report? I would have thought there would be separate dependabot alerts for each version. What does vulnerableRequirements look like when the alert is somehow incorporating multiple versions?

Colm.

@Demaz93
Copy link
Copy Markdown
Contributor Author

Demaz93 commented Feb 2, 2024

I had to blur some data but here is the scenario. As you can see the JSON from GitHub (on the right) has 0.5.34 as vulnerableRequirements but in the code (on the left), I also have 0.5.33 which doesn't have a specific finding opened in GitHub.

image

@coheigea
Copy link
Copy Markdown
Contributor

coheigea commented Feb 2, 2024

IMO if there isn't a specific finding opened in github then we don't need to cater for this scenario when importing findings. All we are doing is importing findings from GitHub after all. Is there no mention of 0.5.33 at all in the results?

@Demaz93
Copy link
Copy Markdown
Contributor Author

Demaz93 commented Feb 2, 2024

No zero mention of 0.5.33.

@Maffooch
Copy link
Copy Markdown
Contributor

Maffooch commented Feb 6, 2024

Hi @Demaz93 I just merged #9462 into the dev branch as it had some unit tests already implemented. Please let me know if that PR does not address your original issue!

@Demaz93 Demaz93 closed this Feb 7, 2024
@Demaz93 Demaz93 deleted the improve_github_parser branch February 7, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants