-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add new parser - Rapplex #10202
base: dev
Are you sure you want to change the base?
Add new parser - Rapplex #10202
Conversation
Hi there 👋, @DryRunSecurity here, below is a summary of our analysis and findings.
Note 🟢 Risk threshold not exceeded. Change Summary (click to expand)The following is a summary of changes in this pull request made by me, your security buddy 🤖. Note that this summary is auto-generated and not meant to be a definitive list of security issues but rather a helpful summary from a security perspective. Summary: The provided code changes cover various aspects of the DefectDojo application, including the addition of a new documentation page for the Rapplex web application security scanner integration, the implementation of a parser for Rapplex scan results, and the update of the application's settings. The documentation change adds a new page that provides information and guidance on how to use the Rapplex scanner with the DefectDojo platform. This change is not directly related to the application's security, but it helps to improve the overall user experience and integration capabilities of the platform. The implementation of the Rapplex parser is a significant security-focused change, as it allows the DefectDojo application to import and process findings from the Rapplex security scanner. The parser extracts relevant information, such as vulnerability details, severity levels, and CWE IDs, which can be used for security analysis and reporting within the DefectDojo platform. The settings update includes several security-related changes, such as the configuration of deduplication algorithms and hashcode calculations for different types of scans, the addition of SAML authentication and remote user authentication options, and the management of file upload types. These changes demonstrate a focus on improving the security and reliability of the DefectDojo application. Additionally, the provided security scan reports highlight the types of vulnerabilities that the Rapplex scanner can identify, including critical SQL Injection, high-severity Cross-Site Scripting (XSS), and various informational and low-severity issues. The inclusion of these sample reports, along with the unit tests for the Rapplex parser, further strengthens the overall security posture of the DefectDojo application. Files Changed:
Powered by DryRun Security |
@AlperenY-cs I just kicked off the tests - they'll need to be green before we'll merge this PR - the Ruff linter has issues you can address now or when the rest of the tests have run. |
Thanks. I solved the ruff linter's problems. #fyi |
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.
Approved
dojo/tools/rapplex/parser.py
Outdated
for reference in issue_definition.get("References", []): | ||
ref_title = reference.get("Title", "") | ||
ref_link = reference.get("Link", "") | ||
reference_texts.append(f"{ref_title}\n{ref_link}") # ref_title and ref_link combined to references section |
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.
reference_texts.append(f"{ref_title}\n{ref_link}") # ref_title and ref_link combined to references section | |
reference_texts.append(f"[{ref_title}]({ref_link})") # ref_title and ref_link combined to references section |
Markdown links would be preferable imo, but feel free to disregard if you disagree
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.
I agree with you, I made it suitable for markdown format.
dojo/tools/rapplex/parser.py
Outdated
desc_sum = issue_definition.get("Sections", {}).get("Summary", "") | ||
|
||
if (len(desc_rem) > 0): | ||
desc_text = f"\n{desc_sum}\nRemediation:\n{desc_rem}" # summary and remediation combined to create description |
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.
DefectDojo offers a field called mitigation
that would be better for the remediation
section of the report
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.
Remediation area assigned to mitigation.
dojo/tools/rapplex/parser.py
Outdated
reference_texts = [] | ||
for reference in issue_definition.get("References", []): | ||
ref_title = reference.get("Title", "") | ||
ref_link = reference.get("Link", "") | ||
reference_texts.append(f"{ref_title}\n{ref_link}") # ref_title and ref_link combined to references section |
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.
I noticed that the Definitions
section is "above" the issues scope. It would be better to collect things like references, summary, remediation, etc. here rather than doing it over and over again in the issues loop. This is a small efficiency, but would be noticeable in larger reporter
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.
I made this change using html2text.
dojo/tools/rapplex/parser.py
Outdated
references=reference_array, | ||
unique_id_from_tool=vuln_id, | ||
) | ||
|
||
finding.active = True |
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.
references=reference_array, | |
unique_id_from_tool=vuln_id, | |
) | |
finding.active = True | |
references=reference_array, | |
unique_id_from_tool=vuln_id, | |
active=True, | |
) |
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.
The active value is now defined in the finding object.
dojo/tools/rapplex/parser.py
Outdated
req = issue.get("HttpRequest", "") | ||
res = issue.get("HttpResponse", "") | ||
vIndex = issue.get("vIndex", "") | ||
vuln_id = f"{scan_id}_{vIndex}" # scanId and vIndex combined to create unique_id_from_tool |
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.
In most tools, the scan ID is used to identify a single instance of a scan. Is this the case here as well? If so, deduplication will never be able to identify a match from scan to scan, as the unique ID from tool is actually TOO unique
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.
Yes, in Rapplex, the scan ID is determined specifically for each scan. unique_id_from_tool is not used now.
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 code changes.
@albay Closing and re-opening as the tests seem stuck for some reason |
Rapplex - Web Application Security Scanner
For more information, Rapplex