API: prevent duplicate saves of taggable entities or when pushing to JIRA
When creating or updating findings (or other entities that have a tags field), the Finding.save() method was called multiple times. Ultimately the finding was saved correctly, but this lead to unnecessary use of resources and could in the future lead to problems if/when the Defect Dojo could be relying more on Django signals such as post_save().
The above was caused by the way the tags fields were implemented long ago. I tried to tweak the TaggitSerializer to prevent the duplicate saves from happening, but this lead nowhere. Then I realized I could just tweak our TagListSerializer to make it work without the "extra" TaggitSerialiser.
I also looked at the from tagulous.contrib.drf import TagSerializer, but that doesn't support the single string with multiple comma separated tags that we re-added support for in #12434
Duplicate saves could also be triggered when the finding needed to be pushed to JIRA. This PR also resolves that.
Added some unit tests to make sure all looks well.
I had to rerecord one JIRA related tests around finding groups. Usually this is a red flag, but looking at the test case and asserts I think all is still good. Maybe the old codebase did some duplicagte/unnecessary/idempotent calls to JIRA?
[sc-10686]
This pull request has conflicts, please resolve those before we can evaluate the pull request.
:red_circle: Risk threshold exceeded.
This pull request contains multiple security vulnerabilities including debug information exposure, verbose error logging that could aid attackers, and a potential CVE ID manipulation risk in different parts of the application's codebase.
Debug Information Exposure in dojo/jira_link/helper.py
| Vulnerability | Debug Information Exposure |
|---|---|
| Description | Debug log messages directly log object contents without sanitization, potentially exposing sensitive internal details if debug logging is enabled in production. |
https://github.com/DefectDojo/django-DefectDojo/blob/7f3d0354ddad5a9ad3c4dd9e169ac16e0fa27c9a/dojo/jira_link/helper.py#L667-L675
Verbose Error Logging in dojo/api_v2/views.py
| Vulnerability | Verbose Error Logging |
|---|---|
| Description | Error messages include the full list of tags and specific invalid tag, which could aid an attacker in enumerating valid tags through targeted error probing. |
https://github.com/DefectDojo/django-DefectDojo/blob/7f3d0354ddad5a9ad3c4dd9e169ac16e0fa27c9a/dojo/api_v2/views.py#L989-L1001
CVE ID Manipulation in dojo/api_v2/serializers.py
| Vulnerability | CVE ID Manipulation |
|---|---|
| Description | The code sets the CVE field by taking the first vulnerability ID from a user-provided list without validation. This could allow an attacker to manipulate the CVE identifier for a finding by providing a crafted list of vulnerability IDs. |
https://github.com/DefectDojo/django-DefectDojo/blob/7f3d0354ddad5a9ad3c4dd9e169ac16e0fa27c9a/dojo/api_v2/serializers.py#L1734-L1765
We've notified @mtesauro.
All finding details can be found in the DryRun Security Dashboard.
Conflicts have been resolved. A maintainer will review the pull request shortly.