-
Notifications
You must be signed in to change notification settings - Fork 1.8k
🎉 Advance reimport to update fix_available field #12633 #12922
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
🎉 Advance reimport to update fix_available field #12633 #12922
Conversation
|
@valentijnscholten is this the right direction for updating fix_available in reimport? |
|
Yes |
|
Shall I target this against bugfix, or do we want a longer testphase and keep it against dev? |
🔴 Risk threshold exceeded.This pull request modifies several sensitive files (dojo/models.py, dojo/importers/default_reimporter.py, dojo/templates/dojo/view_finding.html, dojo/db_migrations/0247_remove_finding_insert_insert_and_more.py) flagged by the configured codepaths scanner and also includes a change in default_reimporter.py where the re-import process updates finding metadata (fix_available and fix_version) without explicit authorization checks, potentially allowing users with only scan import permissions to alter finding metadata. Please verify that these edits are intentional and either restrict the affected codepaths/authors in .dryrunsecurity.yaml or add proper authorization checks to prevent unauthorized metadata modification.
🔴 Configured Codepaths Edit in
|
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/templates/dojo/view_finding.html
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/db_migrations/0247_remove_finding_insert_insert_and_more.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/templates/dojo/view_finding.html
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/models.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/importers/default_reimporter.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/importers/default_reimporter.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/importers/default_reimporter.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/importers/default_reimporter.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/models.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/templates/dojo/view_finding.html
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/models.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/importers/default_reimporter.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/templates/dojo/view_finding.html
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/importers/default_reimporter.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/models.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/models.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
🔴 Configured Codepaths Edit in dojo/models.py
| Vulnerability | Configured Codepaths Edit |
|---|---|
| Description | Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml. |
Unauthorized Modification of Finding Metadata in dojo/importers/default_reimporter.py
| Vulnerability | Unauthorized Modification of Finding Metadata |
|---|---|
| Description | The re-import process, triggered by a user with scan import permissions, updates the fix_available and fix_version fields of an existing finding. This update occurs within the process_matched_active_finding function without performing explicit authorization checks to ensure the user has the necessary permissions to directly edit the finding's metadata. This allows a user with potentially lower privileges (scan import) to modify critical information on a finding that they might not otherwise be authorized to change. |
django-DefectDojo/dojo/importers/default_reimporter.py
Lines 599 to 601 in d82dfdd
| if existing_finding.fix_available != unsaved_finding.fix_available: | |
| existing_finding.fix_available = unsaved_finding.fix_available | |
| existing_finding.fix_version = unsaved_finding.fix_version |
We've notified @mtesauro.
All finding details can be found in the DryRun Security Dashboard.
|
The main reason fields are not updated during reimport is that there's a risk it overwrites fields that were changed by the user. At least that's what I think the reason is. And this could be an issue here are the user may have set the |
|
I mean that is what is requested also in the linked issue explicitly. |
|
We've discussed this and since this is sort of a "status" field or "meta" field it's OK to let
|
I agree with this approach |
7b12ecf to
f32fa7a
Compare
|
Please review @valentijnscholten |
|
Another point @valentijnscholten : |
|
Maybe start with just the |
|
Could we release this for the next release on Monday @mtesauro ? |
c8edad7 to
cc73836
Compare
|
@manuel-sommer I think this PR is a nice addition. Do you need any help to get across? |
|
@valentijnscholten , as there were multiple rebases, the db migration causes these failures. I just rebased again and adapted the db migration, everything should be fine. |
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.
@manuel-sommer Can you regenerated the migrations? This is needed due to the addition of django-pghistory.
just regenerated a new migration @valentijnscholten |
|
I would love to see this being released as I opened the PR already in August. |
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.
Approved. Do you think a small note in 2.52.md should be added to make users aware? It's the first time we let reimport override any non-status fields.
done @valentijnscholten |
Reimport will update existing findings 'fix_available' and 'fix_version' fields based on the incoming scan report.
|
Updated the upgrading notes to 2.53 Just checking in to see if there's a timeline for the review. I'd really appreciate it if this could be merged soon – it would also save me from having to maintain the upgrade notes and db migrations every month. @mtesauro |
#12633 (comment)