Closed Bug 394485 Opened 13 years ago Closed 13 years ago
<object> loads should be checked for malware
bug 384941 adds malware checking to docshell loads, but leaves <object> loads unchecked. These loads should be checked for malware too.
I don't think it should be specific to <object>. I think everything -- iframe, object, script, stylesheet, image -- should be subject to malware checking through the content policy API.
The content policy API is not rich enough to allow that.
I learned from chatting with biesi and dcamp that the malware-site database code lives on a separate thread in order to provide an asynchronous API. That's great because it prevents disk latency from interfering with page load speed (the malware-site database is large enough that it can't just be in memory). But it also means that it can *only* be used asynchronously. I guess the solution to the "hacked ad server" scenario will have to be one of: * Provide the same asynchronous API for <script> loads, so we can fire off the <script> request on a "blocked" HTTP channel that returns nothing until the malware database check has passed. * Maintain a separate "temporarily block all loads from this site" list, and install a content policy hook when this list is non-empty.
I don't see how you can have malware in images or stylesheets, or even scripts. What exactly do we classify as malware and are trying to block here? If someone creates an image that crashes the jpeg decoder and does evilness to it, it seems hard to block that on a uri basis as someone can just move the image. Doesn't <objects> that display webpages create docshells and thereby will use the current malware blocking? Basically, can someone describe what type of attacks we want to prevent, or attach a testcase?
> Doesn't <objects> that display webpages create docshells Yes. > and thereby will use the current malware blocking No, since it doesn't start the load in the docshell. Please see comments in bug 384941.
Assignee: nobody → dcamp
Flags: blocking1.9? → blocking1.9+
I don't know whether I'll be able to review this in a reasonable timeframe. You may want to ask Christian instead.
Attachment #282435 - Flags: review?(bzbarsky) → review?(cbiesinger)
Biesi, are you going to be able to get to this before beta2?
Priority: -- → P3
Attachment #282435 - Flags: review?(cbiesinger) → review?(jonas)
One thing is making me worried, though it's not really introduced with this patch i guess. On redirects you dispatch a runnable to the current thread which is used to suspend the channel. Is there any guarantees that your runnable with run before the channel finishes loading? For example if we're redirected to a cached url?
Comment on attachment 306582 [details] [diff] [review] updated to trunk Looks good otherwise.
Attachment #306582 - Flags: review?(jonas) → review+
(In reply to comment #10) > One thing is making me worried, though it's not really introduced with this > patch i guess. On redirects you dispatch a runnable to the current thread which > is used to suspend the channel. Is there any guarantees that your runnable with > run before the channel finishes loading? For example if we're redirected to a > cached url? This should be safe, at least with the existing set of channels. The Suspend event will be posted to the queue before the new load is started, so will be processed before the pump has a chance to process any stream-ready events. Once suspended, the pump will ignore stream-ready events until unsuspended.
Comment on attachment 306582 [details] [diff] [review] updated to trunk sr=biesi
Attachment #306582 - Flags: superreview+
Checking in content/base/src/nsObjectLoadingContent.cpp; /cvsroot/mozilla/content/base/src/nsObjectLoadingContent.cpp,v <-- nsObjectLoadingContent.cpp new revision: 1.72; previous revision: 1.71 done Checking in content/base/src/nsObjectLoadingContent.h; /cvsroot/mozilla/content/base/src/nsObjectLoadingContent.h,v <-- nsObjectLoadingContent.h new revision: 1.27; previous revision: 1.26 done Checking in docshell/base/Makefile.in; /cvsroot/mozilla/docshell/base/Makefile.in,v <-- Makefile.in new revision: 1.68; previous revision: 1.67 done Checking in docshell/base/nsDocShell.cpp; /cvsroot/mozilla/docshell/base/nsDocShell.cpp,v <-- nsDocShell.cpp new revision: 1.888; previous revision: 1.887 done Checking in docshell/base/nsDocShell.h; /cvsroot/mozilla/docshell/base/nsDocShell.h,v <-- nsDocShell.h new revision: 1.221; previous revision: 1.220 done RCS file: /cvsroot/mozilla/docshell/base/nsIChannelClassifier.idl,v done Checking in docshell/base/nsIChannelClassifier.idl; /cvsroot/mozilla/docshell/base/nsIChannelClassifier.idl,v <-- nsIChannelClassifier.idl initial revision: 1.1 done Checking in docshell/build/nsDocShellCID.h; /cvsroot/mozilla/docshell/build/nsDocShellCID.h,v <-- nsDocShellCID.h new revision: 1.6; previous revision: 1.5 done Checking in docshell/build/nsDocShellModule.cpp; /cvsroot/mozilla/docshell/build/nsDocShellModule.cpp,v <-- nsDocShellModule.cpp new revision: 1.37; previous revision: 1.36 done
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.