Skip to content

Mayhem SARIF support (new parser) #12624

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

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from
Open

Conversation

xansec
Copy link

@xansec xansec commented Jun 18, 2025

Description

This supports parsing Mayhem-generated SARIF reports. In general, the existing SARIF support should work, however, there are some idiosyncrasies as Mayhem is a DAST tool, where the output fields are used in a somewhat non-idiomatic way. Rather than creating a patch change for the existing SARIF report support, I propose this new Mayhem specific support, so that we can keep up with the changes to the SARIF output without burdening or imposing on the existing SARIF parser.

Test results

Tests have been included in dojo/unittests

Documentation

Documentation has been added under dojo/docs/content/en/connecting_your_tools/parsers/file/mayhem.md

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.
  • 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.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

Copy link

dryrunsecurity bot commented Jun 18, 2025

DryRun Security

No security concerns detected in this pull request.


All finding details can be found in the DryRun Security Dashboard.

@xansec
Copy link
Author

xansec commented Jun 18, 2025

I believe the issue raised by the security bot are by design - the SARIF included various examples of exploits, and credentials are known demo credentials for public demo endpoints.

@xansec xansec force-pushed the mayhem-sarif branch 2 times, most recently from c629bcf to b72f2a9 Compare June 18, 2025 04:00
@valentijnscholten
Copy link
Member

Thank you for raising the PR. I just wanted to double check if there's really no way around copying and pasting the existing parser. It's not a lot of code, but I can imagine if SARIF gets more populat we might get more parsers that need slightly different handling of things. Have you considered instead to inherit/extend the existing parser class and overwrite some methods? When I do a "lazy" compare of the SARIF parser with the Mayhem parsers the difference seem very limited, mainly some markdown handling and title cleanup + cwe handling?

@xansec
Copy link
Author

xansec commented Jun 18, 2025

Good catch - I went ahead and extended the SarifParser class instead of copying code. There were a couple of issues with how SarifParser uses member functions vs package functions, so I ended up having to copy code anyways (otherwise polymorphism wasn't working and reverting back to the original SarifParser functions), but I tried to keep this to a minimum. Let me know if there's anything else.

@valentijnscholten
Copy link
Member

Looks like we need to make some changes to the Sarif parser to make it more extensible. That might be too much to try and achieve in this PR. If you could ensure the Mayhem unit tests are testing all the Mayhem specific fields/data, that will help in the future when we make the Sarif parser more extensible.

@valentijnscholten
Copy link
Member

@xansec What do you think about the unit tests? I believe the cwe handling is different from the default SARIF parser, so it's good to have that present in the unit test assertions. What else is different/extended in the Mayhem parser and would be good to assert on in the unit tests?

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.

2 participants