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

Mend SCA Parser update #11395

Merged
merged 55 commits into from
Jan 23, 2025
Merged

Mend SCA Parser update #11395

merged 55 commits into from
Jan 23, 2025

Conversation

testaccount90009
Copy link
Contributor

@testaccount90009 testaccount90009 commented Dec 9, 2024

EDIT - I will publish a different PR with multiple Parser files for one Parser similar to Sonarqube structure. Doing them all in one file is not neat and driving me crazy :). Code findings in API 3.0 / Platform for Mend, SAST respectively.

Minor Platform SCA and Legacy SCA edits.

Part one of this PR is to update 'Locations' and 'File Paths' to try and manage a cleaner way to extract the Path for legacy Mend SCA format vulnerabilities as mentioned here - #11340

Part two of this PR is to remove Locations from Description in the Mend Platform SCA addition I most recently added in 2.40.0.

EDIT - See Edit statement above, but this 'Part 3' for SAST will be contributed after I refactor the parser to have separate files for each, though still retaining one single Parser option. Doing this all in one file is not ideal.
Part three of this PR is to add Mend Platform SAST / 3.0 (Code Security Findings / Vulnerabilities) formatted Findings.

The idea is to create one parser that can accomplish:
-Original Mend Parser SCA Findings preservation. I do not want to update or edit this here much, since people may already be using this in the wild. Previously I added an edit to the Locations / File Paths, but due to the issue linked above; it may negatively impact teams who have a large list of file paths / locations with their Findings from the Mend Legacy SCA (non-platform) output / legacy 1.0 API formatted output.

-New Platform SCA parser. This was implemented in my most recent PR affecting Mend parser.py, however an oversight of adding the Filepath to the Description has me wanting to refactor the Location out of the Description, as to not affect deduplication since Description and Title are used.

EDIT - See above Edits, this isn't in this PR at this moment.
-New Platform SAST parser. This will be the 'meat' and main portion of this PR. Code SAST Findings in Mend should be able to be identified and structured as a result of changes in this PR.

Checklist

This checklist is for your information.

  • Make sure to rebase your PR against the very latest dev.
  • Features/Changes should be submitted against the dev.
  • Bugfixes should be submitted against the bugfix branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is flake8 compliant.
  • Your code is python 3.11 compliant.
  • If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
  • Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

Extra information

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

Moderators: Labels currently accepted for PRs:

  • Import Scans (for new scanners/importers)
  • enhancement
  • performance
  • feature
  • bugfix
  • maintenance (a.k.a chores)
  • dependencies
  • New Migration (when the PR introduces a DB migration)
  • settings_changes (when the PR introduces changes or new settings in settings.dist.py)

Contributors: Git Tips

Rebase on dev branch

If the dev branch has changed since you started working on it, please rebase your work after the current dev.

On your working branch mybranch:

git rebase dev mybranch

In case of conflict:

 git mergetool
 git rebase --continue

When everything's fine on your local branch, force push to your myOrigin remote:

git push myOrigin --force-with-lease

To cancel everything:

git rebase --abort

Squashing commits

git rebase -i origin/dev
  • Replace pick by fixup on the commits you want squashed out
  • Replace pick by reword on the first commit if you want to change the commit message
  • Save the file and quit your editor

Force push to your myOrigin remote:

git push myOrigin --force-with-lease

Code findings in API 3.0 / Platform for Mend, SAST respectively.
Copy link

dryrunsecurity bot commented Dec 9, 2024

DryRun Security Summary

The pull request enhances the Mend security scanner parser in DefectDojo by improving report format handling, location processing, inactive finding filtering, and finding deduplication to increase the accuracy and reliability of security vulnerability reporting.

Expand for full summary

Summary:

The code changes in this pull request focus on improving the functionality of the Mend security
scanner parser in the DefectDojo application. The key changes include:

  1. Handling Different Report Formats: The parser can now handle reports generated from the Mend CLI, Mend Platform, and the Mend 3.0 API, allowing for greater flexibility and compatibility.

  2. Improving Location Handling: The code now handles the "locations" field in the Mend report more robustly, truncating the location information if it exceeds the maximum length to avoid issues with the DefectDojo Finding model.

  3. Handling Inactive Findings: The parser now filters out inactive findings from the Mend report, ensuring that only active vulnerabilities are added to the DefectDojo application.

  4. Deduplication: The code includes a function to hash the finding's description and title to retrieve a key for deduplication, helping to avoid creating duplicate findings in the DefectDojo application.

Additionally, the changes to the unit tests indicate that the parser is now providing more detailed and informative output for the identified vulnerabilities, including the vulnerability identifier, affected library, and the location where the vulnerable component was found.

These improvements to the Mend parser functionality are positive from an application security perspective, as they enhance the accuracy and reliability of the security findings reported in the DefectDojo application.

Files Changed:

  1. dojo/tools/mend/parser.py: This file contains the core functionality of the Mend security scanner parser. The changes made in this pull request focus on improving the handling of different report formats, location information, inactive findings, and deduplication of findings.

  2. unittests/tools/test_mend_parser.py: This file contains the unit tests for the Mend parser. The changes made in this pull request update the test cases to reflect the improvements in the parser's output, including more detailed information about the identified vulnerabilities.

Code Analysis

We ran 9 analyzers against 3 files and 0 analyzers had findings. 9 analyzers had no findings.

View PR in the DryRun Dashboard.

Remove locations / path from Description of SCA Platform output and instead implement locations in steps_to_reproduce.
Updating value to a placeholder severityRating right now of 2.143.  Still working on this and the cvssv3 assertion values.
@testaccount90009 testaccount90009 changed the title Mend Platform SAST + SCA Parser update Mend SCA Parser update Dec 9, 2024
@testaccount90009
Copy link
Contributor Author

testaccount90009 commented Dec 9, 2024

To be clear, this is only for SCA and fixing Locations + removing the Library Path from the description to avoid being used in deduplication.

The mentioned SAST changes will be in a future PR after I refactor and clean up this so I can break out a single parser into multiple files, instead of attempting to code everything in one file.

I will refactor like this structure -
https://github.com/DefectDojo/django-DefectDojo/tree/dev/dojo/tools/sonarqube

Instead of one single parser.py file attempting to construct each output case.

@testaccount90009
Copy link
Contributor Author

I am testing these changes in my dev stack right now.

@testaccount90009
Copy link
Contributor Author

After testing import on Legacy SCA and Platform SCA:

Legacy - These changes fix the varchar 4000 issue for the file paths field for legacy SCA vulns, while adding it into steps_to_reproduce successfully. The file_paths is no longer used for Legacy SCA and can no longer error out on the var char 4000 limit for that field in the DB/model.

Platform - Adding logic for steps_to_reproduce to retrieve the component.path and Transitive dependencies should now include the component -> path as the steps_to_reproduce. If the library and vulnerability is Direct, then Path is not in the Mend SCA Platform output json file, only for Transitive ones for some reason. Maybe they (Mend) will update this at some point, but it's still good to gather the component path the vulnerability is in and whenever they get around to adding Direct dependencies with the component path data, it will be available in the Finding in DefectDojo with this parser change.\

I am pretty sure this is the final Legacy / SCA Platform parser change I am setting out to accomplish, before my next PR which will break this out similar to a structure as defined in the sonarqube tool parser. One parser file to handle the context of multiple schemas is not the cleanest approach, and after talking with my team - we agreed that before I introduce the Platform SAST parser capabilities, it would be best to restructure the Mend parser files to be similar to that of SonarQube and making the logic a bit easier to follow, when broken out.

@testaccount90009
Copy link
Contributor Author

TL:DR - They're aware of their inconsistent data which is what caused this issue - I added a check to the parser code to only grab findingInfo -> status == ACTIVE findings. The parser file for multiple-findings.json contains totalItems = 11, but should only be 5 Active. I've updated the unit test to reflect that as well.

I will run a test for this fix locally on my dev stack on Monday and confirm if this fixed the issue or not.

Per Mend: Legacy UI and API show matched and consistent data with no concept of a finding being remediated. Mend does an override for a scan inventory (though you can specify append) which removes all of the libraries from the inventory and the vulnerabilities related to those libraries.

However... in Mend Platform UI this is also true, but NOT for the API... The API can return remediated 'LIBRARY_REMOVED' (findingInfo -> status) Findings and vulnerabilities. Why their UI doesn't show this - no clue.. Why their API docs don't document this - no clue.. Their API and UI are still inconsistent (UI should show remediated Findings then if they're sticking around for history), so I am going to ask Mend to clarify their 3.0 API Documentation, as well as update their UI to show "LIBRARY_REMOVED" Findings as well.

Quite frankly, I'd prefer if their 3.0 Platform API just removed the findings from the json 'getProjectSecurityDependencyFindings' call, but I guess they feel the need to keep those around in the API (but not present them in the UI)... A head scratcher for sure!

@testaccount90009
Copy link
Contributor Author

I will test this in my dev stack on Monday, but I believe this is an iteration of the parser that can parse ACTIVE only Findings. I also updated the unit tests to make sure title was coming across correctly.

@testaccount90009
Copy link
Contributor Author

@Maffooch I can confirm there are no longer inaccurate / inconsistent entries for Findings that are no longer 'ACTIVE' status from Mend SCA Platform 3.0 API. With the latest updates and after the response from the Mend team, the parser only retrieves ACTIVE findings and no longer retrieves 'LIBRARY_REMOVED' or other status Findings.

Copy link
Contributor Author

@testaccount90009 testaccount90009 left a comment

Choose a reason for hiding this comment

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

I have been running this in both Dev and Prod stacks for a little over a week and it looks like everything is working as intended, after working with Mend support on the resolution. They have no filter for their API call to retrieve only ACTIVE findings, so I added in the parser to only look for ACTIVE status findings. Mend is working on their inconsistencies between the UI and API. UI only shows Active or Suppressed findings, but no "library_removed" ones, whereas the API contains all three (including library_removed) in the Dependency findings output. Now the Platform findings also contain 'Impact' as the Direct or Transitive dependency association, in addition to the "Steps to Reproduce" providing the Location of the offending library / file. Note: there is a separate bug within Mend Platform that in some conditions is missing the Location. This doesn't break anything, it's just another bug in Mend that they're aware of and working on, due to their Platform integration. When Mend fixes it, it will no longer be missing for certain edge cases - not a huge deal.

Please let me know if there's any other changes I should make. @Maffooch

Copy link
Contributor

@Maffooch Maffooch left a comment

Choose a reason for hiding this comment

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

Thanks for your work here @testaccount90009 😄

@testaccount90009
Copy link
Contributor Author

Happy to contribute!

@mtesauro
Copy link
Contributor

mtesauro commented Jan 5, 2025

Thanks for your work here @testaccount90009 😄

+1 on ☝️

@testaccount90009
Copy link
Contributor Author

@mtesauro happy to help! This data for our company has been huge already in identifying Direct dependencies and their vulnerabilities vs Transitive ones, in addition to their location.

Please let me know if there's any other changes I should make before this gets merged in.

@testaccount90009
Copy link
Contributor Author

@cneill any suggestions in this PR?

@mtesauro mtesauro merged commit c2ed251 into DefectDojo:dev Jan 23, 2025
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants