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

Potentially Unwanted Programs Scan #37

Closed
bitnom opened this issue Nov 29, 2018 · 4 comments · Fixed by #175
Closed

Potentially Unwanted Programs Scan #37

bitnom opened this issue Nov 29, 2018 · 4 comments · Fixed by #175

Comments

@bitnom
Copy link

bitnom commented Nov 29, 2018

Currently, browsers perform a scan on files when downloaded. I'm not sure how this works under the hood but I know that some browsers have their own internal scanners. Also, that under most operating systems, the default virus/malware scanner is invoked in any newly downloaded path. Any such hooks should also be applied to paths newly created by this API.

@mkruisselbrink
Copy link
Contributor

This is definitely something we're considering. A few issues with it though: in chrome, for downloads, the way these checks work is that the file is first downloaded to a temporary file, then checked and finally moved/renamed to its actual destination. That flow won't really work if we want to support an efficient pwrite style API (#34) that lets a webapp modify individual bytes of files. Another problem with chrome's downloaded file checking is that it doesn't really work offline, and we explicitly do want this API to work offline as well.

So yeah, it would make sense to have some kind of safe browsing/virus scan checks as part of this, but I'm not sure I see a way of doing that while also keeping a reasonably performant API that actually solves the use cases we want to solve. Perhaps some balance is to only support pwrite style writing for "safe" file types (and definitely not for anything that might be executable), and restricting more dangerous file types to a "write this single Blob to the file" API.

@DanielHerr
Copy link

Is there a reason Chrome or the operating system cannot scan the file after writing has been completed, or before the user attempts to open it?

@mkruisselbrink
Copy link
Contributor

"After writing has been completed" might not be very well defined with a pwrite style API. Doing a check after every individual write would probably cause unacceptable performance overhead, but doing the checks later after some delay/some amount of "idle" time could result in a bad user experience where chrome first tells the website that the write was successful (and the website probably then tells the user), but ultimately we end up somehow acting on detected possible malware.

Also unclear how we'd react on anything detected in such a way. Simply deleting the suspected malware files could be problematic, especially if a website is merely modifying an existing file on disk. Maybe we could try to undo the modifications, but that isn't really plausible either (without deep OS integration on certain types of filesystems only).

@DanielHerr
Copy link

Are you aware of what happens with malware detected when the user selects an existing file to overwrite using the download functionality?

mkruisselbrink added a commit that referenced this issue May 15, 2019
This attempts to solve #37 by making it explicit that changes made to files
might not be reflected on disk until flush() or close() is called on the
writer.

This also introduces a "atomic" mode for FileSystemWriter where any changes
are written to a temporary file, and only on flush or close is that file
moved on top of the destination file. This should address #48.
mkruisselbrink added a commit that referenced this issue Jun 10, 2019
This attempts to solve #37 by making it explicit that changes made to files
might not be reflected on disk until flush() or close() is called on the
writer.

This also introduces a "atomic" mode for FileSystemWriter where any changes
are written to a temporary file, and only on flush or close is that file
moved on top of the destination file. This should address #48.
mkruisselbrink added a commit that referenced this issue Jun 10, 2019
* Add options to createWriter() to deal with atomic writes.

This attempts to solve #37 by making it explicit that changes made to files
might not be reflected on disk until close() is called on the
writer.

This also introduces a "atomic" mode for FileSystemWriter where any changes
are written to a temporary file, and only on close is that file
moved on top of the destination file. This should address #48.
mkruisselbrink added a commit that referenced this issue Apr 16, 2020
This makes it clear when user agents can perform such scans, and how
failing a check will be reported.

Fixes #37
mkruisselbrink added a commit that referenced this issue May 5, 2020
This makes it clear when user agents can perform such scans, and how
failing a check will be reported.

Fixes #37
mkruisselbrink added a commit that referenced this issue May 5, 2020
* Call out malware scans in writer close algorithm.

This makes it clear when user agents can perform such scans, and how
failing a check will be reported.

Fixes #37

* address comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants