Closed Bug 776797 Opened 7 years ago Closed 7 years ago

Lock down POfflineCacheUpdate

Categories

(Core :: Networking: Cache, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18
blocking-basecamp +

People

(Reporter: cjones, Assigned: mayhemer)

References

(Blocks 1 open bug)

Details

(Whiteboard: [WebAPI:P2][LOE:S])

Attachments

(1 file)

Right now we trust the data in the request, but we should only allow updating resources owned by the current origin loaded in the PBrowser.
Can you please be more specific?
blocking-basecamp: --- → +
blocking-basecamp: + → ?
Whiteboard: [blocked-on-input]
I forget why we're blocked on input here.  Can anyone enlighten me?
Needs jduell's analysis to determine if there's work here or not.
Assignee: nobody → jduell.mcbugs
blocking-basecamp: ? → +
Whiteboard: [blocked-on-input]
Whiteboard: [WebAPI:P2]
Keywords: feature
From talking to Honza, this appears to be just a matter of checking here

  http://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabParent.cpp#1046

that both the aManifestURI, aDocumentURI are from the origin the app is from (using the app info we can get from the TabParent itself) and that the app has permission to use the offline cache.
Assignee: jduell.mcbugs → honzab.moz
Whiteboard: [WebAPI:P2] → [WebAPI:P2][LOE:S]
Just to confirm and repeat:

- check the documentURI and the manifestURI are the same origin
- check that for domain of the manifestURI we allow offline caching (nsOfflineCacheUpdateService::OfflineAppAllowedForURI(aManifestURI))

This will not break anything, since manifest URI must always be the same origin as the document URI we originally do the permission check for in nsDocShell::LoadURI (or one of its internal methods).
Status: NEW → ASSIGNED
Attached patch v1Splinter Review
Attachment #665099 - Flags: review?(jduell.mcbugs)
Comment on attachment 665099 [details] [diff] [review]
v1

Review of attachment 665099 [details] [diff] [review]:
-----------------------------------------------------------------

::: uriloader/prefetch/OfflineCacheUpdateChild.cpp
@@ +54,5 @@
>  // OfflineCacheUpdateChild::nsISupports
>  //-----------------------------------------------------------------------------
>  
>  NS_INTERFACE_MAP_BEGIN(OfflineCacheUpdateChild)
> +  NS_INTERFACE_MAP_ENTRY(nsISupports)

I assume this is an unrelated fix?  Why do you need an entry for nsISupports?  (nsHttpChannel, for instance, doesn't have it in its map)
Attachment #665099 - Flags: review?(jduell.mcbugs) → review+
(In reply to Jason Duell (:jduell) from comment #8)
> > +  NS_INTERFACE_MAP_ENTRY(nsISupports)
> 
> I assume this is an unrelated fix?  Why do you need an entry for
> nsISupports?  (nsHttpChannel, for instance, doesn't have it in its map)

Thanks for r.  This is needed, otherwise QI asserts here [1] when invoked from some IPC code.  I think there is always a need for nsISupports.

According nsHttpChannel: it inherits it from HttpBaseChannel ;)

[1] http://hg.mozilla.org/mozilla-central/annotate/aacf4867f830/xpcom/glue/nsISupportsImpl.h#l690
This should probably never have had the feature keyword.  Sorry for the noise.
Keywords: feature
https://hg.mozilla.org/mozilla-central/rev/42200a47baea
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.