Skip to content
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

Malware check for attachments #2905

Closed
jaakkocsc opened this issue Apr 5, 2022 · 2 comments
Closed

Malware check for attachments #2905

jaakkocsc opened this issue Apr 5, 2022 · 2 comments
Labels
Stat Statistics Finland Technical Debt Improving internal product quality

Comments

@jaakkocsc
Copy link
Collaborator

REMS does not perform malware scanning to the uploaded files. An attacker can upload a malicious file to the server. If the malicious file is run by a handler, the user's device can get infected, and the malware can spread.

@jaakkocsc jaakkocsc created this issue from a note in Rems task board (User Feedback) Apr 5, 2022
@jaakkocsc jaakkocsc added the Stat Statistics Finland label Apr 5, 2022
@jaakkocsc jaakkocsc added the Technical Debt Improving internal product quality label Apr 25, 2022
@hallundbaek
Copy link
Contributor

This feature is also relevant for compliance with regulatory requirements in Denmark.

I will probably take a stab at a PR in the coming days.

My initial idea for an MVP of this feature goes something like this:

  • Add config option, something like :malware-scanner-path, that would contain a path to an executable
  • The executable would be expected to be able to ingest binary data from stdin, and return with a 0 exit code in case of clean file, and a non-zero exit code otherwise.
  • In the case of a clean file, the request will be processed as usual
  • Otherwise the file will not be persisted, the incident will be logged, and the request get a negative response.
  • Minimal UI to relay the information that the uploaded file flagged and that the upload has failed.

This could be extended in the future to include:

  • A table of quarantined blobs, for use in investigative security.
  • An optional requirement on the malware-scanner executable to produce a plaintext report on stdout, which enables:
  • The possibility to receive and log a malware scan report.
  • The possibility to relay malware scan report to user interface.

Thoughts?

@Macroz
Copy link
Collaborator

Macroz commented Sep 29, 2023

Sounds pretty good.

I think for the very first version:

  • config
  • when attachment has been uploaded, scan it synchronously, and then upload to table or give error result
  • add to the tests at least one with a dummy scanner (could be e.g. shell script)

I think we have everything setup so that the attachment is first uploaded to a temporary file, so that may be available for the scanner so stdin passing may not be required. It's something to check.

It could be that sometimes the scan is slow, so ideally the architecture may be something like:

  • attachment uploaded
  • record attachment as empty to application
  • UI will show the attachment but trying to download it will say it's not checked yet
  • later, a background job starts to scan the file
  • later, background job stores the correct bytes into the attachment table if OK
  • attachment data is now downloadable from UI

But I would do the simple version first.

@aatkin aatkin moved this from User Feedback to In Progress in Rems task board Nov 8, 2023
@Macroz Macroz closed this as completed Nov 13, 2023
@Macroz Macroz moved this from In Progress to Done (newest on top) in Rems task board Nov 13, 2023
@meericsc meericsc moved this from Done (newest on top) to Accepted in Rems task board Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stat Statistics Finland Technical Debt Improving internal product quality
Projects
Archived in project
Development

No branches or pull requests

3 participants