Closed Bug 394485 Opened 12 years ago Closed 12 years ago

<object> loads should be checked for malware

Categories

(Core :: DOM: Core & HTML, defect, P2)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: dcamp, Assigned: dcamp)

Details

(Whiteboard: [sg:want P2])

Attachments

(1 file, 1 obsolete file)

bug 384941 adds malware checking to docshell loads, but leaves <object> loads unchecked.  These loads should be checked for malware too.
Flags: blocking1.9?
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+
Attachment #282435 - Flags: review?(bzbarsky)
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)
Version: unspecified → Trunk
Biesi, are you going to be able to get to this before beta2?
Priority: -- → P3
Flags: tracking1.9+ → blocking1.9+
Priority: P3 → P2
Attachment #282435 - Flags: review?(cbiesinger) → review?(jonas)
Attached patch updated to trunkSplinter Review
Attachment #282435 - Attachment is obsolete: true
Attachment #306582 - Flags: review?(jonas)
Attachment #282435 - Flags: 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: 12 years ago
Resolution: --- → FIXED
Whiteboard: [sg:want P2]
Component: DOM: Core → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.