dfir-iris / iris-web

Collaborative Incident Response platform
GNU Lesser General Public License v3.0
1.08k stars 184 forks source link

[BUG] Mass Delete of Alerts - HTTP 500 Unhashable List #581

Closed bvirgilioamnh closed 1 month ago

bvirgilioamnh commented 2 months ago

Describe the bug When attempting to delete multiple alerts via the bulk selection the server errors out and the alerts are never deleted. The close alert functionality works fine.

To Reproduce Steps to reproduce the behavior:

  1. Select multiple alerts.
  2. Delete alerts
  3. Receive error

Expected behavior Deletion of selected alerts.

Screenshots

image
iriswebapp_app       | 2024-09-11 12:11:49 :: ERROR :: app :: log_exception :: Exception on /alerts/batch/delete [POST]
iriswebapp_app       | Traceback (most recent call last):
iriswebapp_app       |   File "/opt/venv/lib/python3.9/site-packages/flask/app.py", line 2190, in wsgi_app
iriswebapp_app       |     response = self.full_dispatch_request()
iriswebapp_app       |   File "/opt/venv/lib/python3.9/site-packages/flask/app.py", line 1486, in full_dispatch_request
iriswebapp_app       |     rv = self.handle_user_exception(e)
iriswebapp_app       |   File "/opt/venv/lib/python3.9/site-packages/flask/app.py", line 1484, in full_dispatch_request
iriswebapp_app       |     rv = self.dispatch_request()
iriswebapp_app       |   File "/opt/venv/lib/python3.9/site-packages/flask/app.py", line 1469, in dispatch_request
iriswebapp_app       |     return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
iriswebapp_app       |   File "/iriswebapp/app/util.py", line 741, in wrap
iriswebapp_app       |     return f(*args, **kwargs)
iriswebapp_app       |   File "/iriswebapp/app/blueprints/alerts/alerts_routes.py", line 497, in alerts_batch_delete_route
iriswebapp_app       |     alert = call_modules_hook('on_postload_alert_delete', data={"alert_ids": {alert_ids}})
iriswebapp_app       | TypeError: unhashable type: 'list'

Desktop (please complete the following information): N/A

Smartphone (please complete the following information): N/A

Additional context None currently

bvirgilioamnh commented 2 months ago

This is the problematic line: https://github.com/dfir-iris/iris-web/blob/7123a2ac6ab399648e7e4b940dbea766cdb0e42f/source/app/blueprints/alerts/alerts_routes.py#L497C1-L497C91

I'm not sure the "proper" fix here but the other batch routes iterate over each alert and call the module hook each time:

alert = call_modules_hook('on_postload_alert_update', data=alert)

whereas the delete just passes all the alert_ids:

   alert = call_modules_hook('on_postload_alert_delete', data={"alert_ids": {alert_ids}})

Possible fix might be to wrap the hook call in a loop like the others or remove the {} wrapper on alert_ids and pass the original list of alert IDs. I don't currently have a development environment setup, but this looks like it could be an easy fix.

bvirgilioamnh commented 2 months ago

Looking at the git blame for that line, I think commit c6d6f3e7abe0ba7f0dce605765c56825f3f1c46c by @whikernel erroneously removed the format string which rendered the alert_ids as the list.

I think the correct solution is to indeed remove the {} on alert_ids and just pass it as a variable as this is how it was originally being handled (albeit still incorrectly based on the changes in the commit).

whikernel commented 1 month ago

This is now fixed in v2.4.13