-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add OpenReports import support #13562
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
base: dev
Are you sure you want to change the base?
Conversation
|
This pull request introduces a potential resource exhaustion (Denial of Service) issue: the OpenReports parser reads the entire uploaded JSON report into memory via scan_file.read() and json.loads(), which can consume large amounts of memory for big or malicious files and crash worker processes. Consider streaming or incremental parsing, enforcing strict upload size limits, or validating/sanitizing input before full parsing to mitigate the risk.
Resource Exhaustion (Denial of Service) in
|
| Vulnerability | Resource Exhaustion (Denial of Service) |
|---|---|
| Description | The OpenReports parser reads the entire uploaded JSON report file into memory using scan_file.read() and then json.loads(). This process can lead to significant memory consumption, especially with large or maliciously crafted JSON files, potentially causing a Denial of Service (DoS) by exhausting the application's memory resources and crashing the worker process. Even if there are file upload size limits, the memory amplification during JSON parsing can still be substantial. |
django-DefectDojo/dojo/tools/openreports/parser.py
Lines 47 to 50 in f31f60f
| data = json.loads(str(scan_data, "utf-8")) | |
| except Exception: | |
| data = json.loads(scan_data) | |
All finding details can be found in the DryRun Security Dashboard.
valentijnscholten
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! It looks quite good already. Some questions:
- Can you add at least one vulnerability for which the policy field is not a CVE?
- Could you look at the hash_code configuration for deduplication? It might be good to check wha the best fit ir or if there's a field that we can use as a value for
unique_id_from_tool.
manuel-sommer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add a documentation file for the parser.
8cbb5ca to
7647272
Compare
|
I think also if this is a generic report format similar to SARIF, the openreports parser should do something similar where the actual report type / scanner type ends up in the test name in Defect Dojo. django-DefectDojo/dojo/tools/sarif/parser.py Lines 113 to 117 in 16c749c
|
01dc290 to
53ef8bb
Compare
valentijnscholten
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for extending the PR. The values for the scanner names seem a bit generic and may change in the future if more operators/scanners will use this output format, but that's outside our control.
valentijnscholten
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, forgot that there were some small issues to address.
|
|
||
| # Set vuln_id_from_tool to the policy field for deduplication | ||
| # This allows using DEDUPE_ALGO_UNIQUE_ID_FROM_TOOL_OR_HASH_CODE | ||
| finding.vuln_id_from_tool = policy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@valentijnscholten Will this work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the field is unique_id_from_tool. But it must be unique per finding and across multiple scans/import.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I am blind. Then the correct way would be hashing, like you previously mentioned.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
Add OpenReports JSON Import Parser
Description
Adds support for importing scans in OpenReports.io format. Currently only statnett operator outputs trivy scans in this format, but being an open standard backed by e.g kyverno, more tools could adopt it.
More information can be found here: OpenReports.io
Steps to get the relevant file:
kubectl get reports -ojson -A > reports.jsonFeatures:
Test results
Added unit tests covering empty results, single reports, list format, and parser metadata. All tests pass with sample JSON files included.
I have also tested this by importing the resulting JSON file in the DefectDojo GUI and this works.
Documentation
Parser follows standard DefectDojo interface with inline documentation and sample files.
Checklist