DefectDojo / django-DefectDojo

DevSecOps, ASPM, Vulnerability Management. All on one platform.
https://defectdojo.com
BSD 3-Clause "New" or "Revised" License
3.71k stars 1.56k forks source link

GitLab SAST importer, incorrect nums of Findings #5749

Closed zakrush closed 2 years ago

zakrush commented 2 years ago

Be informative GItLab SAST that containt 2 vulnerability parsing as one vuln.

Bug description Incorrect nums of Findings for Gitlab SAST report

Steps to reproduce Steps to reproduce the behavior:

  1. Create Egagment
  2. Import Gitlab SAST report (attached. Rename to json before import)
  3. See 1 findings instead 2 findings. Atached json have two vunerabilities fields

Expected behavior 2 different findings was created.

Deployment method (select with an X)

Environment information

Logs Use docker-compose logs here no errors

Attach debug logs:

defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.importers.importer.importer:284] IMPORT_SCAN: parameters: {'self': <dojo.importers.importer.importer.DojoDefaultImporter object at 0x7f4cb169af10>, 'scan': <TemporaryUploadedFile: GL-new_sql.json (application/json)>, 'scan_type': 'GitLab SAST Report', 'engagement': <Engagement: Engagement: test_semgrep_default (Jan 13, 2022)>, 'lead': <SimpleLazyObject: <User: admin>>, 'environment': <Development_Environment: Default>, 'active': True, 'verified': True, 'tags': [], 'minimum_severity': 'Info', 'user': None, 'endpoints_to_add': [], 'scan_date': datetime.datetime(2022, 1, 14, 0, 0, tzinfo=<UTC>), 'version': '', 'branch_tag': '', 'build_id': '', 'commit_hash': '', 'push_to_jira': None, 'close_old_findings': False, 'group_by': '', 'api_scan_configuration': None, 'service': ''}
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.importers.importer.importer:303] IMPORT_SCAN parser v2: Create Test and parse findings
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.importers.importer.importer:331] IMPORT_SCAN parser v2: Parse findings (aggregate)
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.importers.importer.importer:351] IMPORT_SCAN: Processing findings
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.importers.importer.importer:66] endpoints_to_add: []
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.importers.importer.importer:69] starting import of 1 items.
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.finding.helper:60] changed fields: {'id': (None, None)}
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.models:2385] Saving finding of id None dedupe_option:False (self.pk is None)
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.decorators:139] model_or_id: Untrusted Input Concatinated With Raw SQL Query Can Result in SQL Injection.
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.decorators:26] user: admin
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.decorators:32] dojo_async_task <function post_process_finding_save at 0x7f4cb3f6c8b0>: no current user, running task in the background
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.decorators:66] converting model_or_id to id: Untrusted Input Concatinated With Raw SQL Query Can Result in SQL Injection.
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.decorators:26] user: admin
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.decorators:32] dojo_async_task <@task: dojo.finding.helper.post_process_finding_save of dojo at 0x7f4cd0caeac0>: no current user, running task in the background
defectdojo-celeryworker-1  | [14/Jan/2022 13:12:36] INFO [celery.worker.strategy:155] Task dojo.finding.helper.post_process_finding_save[9c1c679f-7dc2-49a9-a25c-44bdf3b16dde] received
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.specific-loggers.deduplication:2028] No configuration for hash_code computation found; using default fields for static scanners
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.specific-loggers.deduplication:2051] compute_hash_code_legacy - fields_to_hash = Untrusted Input Concatinated With Raw SQL Query Can Result in SQL Injection.None880ImportDirectoryController.jsScanner: Semgrep
defectdojo-uwsgi-1         | Untrusted input concatinated with raw SQL query can result in SQL Injection.
defectdojo-uwsgi-1         |
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.models:2100] fields_to_hash      : Untrusted Input Concatinated With Raw SQL Query Can Result in SQL Injection.None880ImportDirectoryController.jsScanner: Semgrep
defectdojo-uwsgi-1         | Untrusted input concatinated with raw SQL query can result in SQL Injection.
defectdojo-uwsgi-1         |
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.models:2101] fields_to_hash lower: untrusted input concatinated with raw sql query can result in sql injection.none880importdirectorycontroller.jsscanner: semgrep
defectdojo-uwsgi-1         | untrusted input concatinated with raw sql query can result in sql injection.
defectdojo-uwsgi-1         |
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.specific-loggers.deduplication:2358] Hash_code computed for finding: a3a2b65c21c71a10464aab2f82527079011d82dd5942872a2fde0db6d39c63ec
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.models:2385] Saving finding of id 769 dedupe_option:True (self.pk is not None)
defectdojo-celeryworker-1  | [14/Jan/2022 13:12:36] INFO [celery.app.trace:131] Task dojo.finding.helper.post_process_finding_save[9c1c679f-7dc2-49a9-a25c-44bdf3b16dde] succeeded in 0.006826476994319819s: None
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.finding.helper:41] 769: changed status fields pre_save: {'id': (None, 769)}
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.finding.helper:44] 769: id changed from None to 769
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.finding.helper:60] changed fields: {'id': (None, 769)}
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.decorators:139] model_or_id: Untrusted Input Concatinated With Raw SQL Query Can Result in SQL Injection.
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.decorators:26] user: admin
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.decorators:32] dojo_async_task <function post_process_finding_save at 0x7f4cb3f6c8b0>: no current user, running task in the background
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.decorators:66] converting model_or_id to id: Untrusted Input Concatinated With Raw SQL Query Can Result in SQL Injection.
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.decorators:26] user: admin
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.decorators:32] dojo_async_task <@task: dojo.finding.helper.post_process_finding_save of dojo at 0x7f4cd0caeac0>: no current user, running task in the background
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.importers.importer.importer:362] IMPORT_SCAN: Updating test/engagement timestamps
defectdojo-celeryworker-1  | [14/Jan/2022 13:12:36] INFO [celery.worker.strategy:155] Task dojo.finding.helper.post_process_finding_save[57b7c80b-e53c-49ef-afea-8dad02032bd3] received
defectdojo-celeryworker-1  | [14/Jan/2022 13:12:36] INFO [celery.app.trace:131] Task dojo.finding.helper.post_process_finding_save[57b7c80b-e53c-49ef-afea-8dad02032bd3] succeeded in 0.013314529998751823s: None
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.importers.importer.importer:366] IMPORT_SCAN: Updating Import History
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.importers.utils:36] new: 1 closed: 0 reactivated: 0
defectdojo-uwsgi-1         | [14/Jan/2022 13:12:36] DEBUG [dojo.importers.utils:58] preparing Test_Import_Finding_Action for finding: 769

Sample scan files GL-new_sql.txt

Screenshots I have only this finding: image

Additional context (optional) Add any other context about the problem here.

damiencarol commented 2 years ago

@zakrush your report have 2 vulnerabilities but they have most of there attributes with the same value (title, severity, file, etc) So the default hash algorithm see them equals.

zakrush commented 2 years ago

@damiencarol thank you. I miss this, I'm so sorry. Close this issue.

zakrush commented 2 years ago

@damiencarol sorry again. https://github.com/DefectDojo/django-DefectDojo/blob/e77427d401c7bf9d29b056e76d902638a8416c5c/dojo/tools/gitlab_sast/parser.py#L63 Maybe modify this? Becouse into my case we are losing some finding, that incorrect as I think. Maybe are using list for items?

damiencarol commented 2 years ago

@zakrush you should take a look at our de-duplication algorithm documentation

zakrush commented 2 years ago

@damiencarol I see this. I setup unique_id_from_tools for it.

In this case, when vulnerabilities is parsing, the first vulns is overrided the second vuln with the same ID. Code is here: https://github.com/DefectDojo/django-DefectDojo/blob/e77427d401c7bf9d29b056e76d902638a8416c5c/dojo/tools/gitlab_sast/parser.py#L60-L64

In this case I think DD is should parse my example as 2 findings, and dedup it as describe to settings.dist.py. (in'my case i should get 2 findings. One of them is duplicated marked)

valentijnscholten commented 2 years ago

Looks like the id field in the Gitlab report isn't actually unique, so we cannot use it as a key in our dictionary in the parser.

zakrush commented 2 years ago

@damiencarol Yes. Is it. It's bug of semgrep. I create issue for this tool. https://github.com/returntocorp/semgrep-action/issues/488

damiencarol commented 2 years ago

@zakrush any news on this one? Have you been able to confirm that it's the tool that produce wrong data in the report?

zakrush commented 2 years ago

Yes. It's tool are given incorect data. Semgrep-agent is hashed data defined only by namespace and path, that incorrect for SAST tools

Is it id. str(uuid.uuid5(uuid.NAMESPACE_URL, str(self.path))),

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.