[webkit-dev] Blob changes to SecurityOrigin.cpp

[Jian Li]

On Fri, Sep 3, 2010 at 3:43 PM, Adam Barth <abarth at webkit.org> wrote:

> On Fri, Sep 3, 2010 at 3:19 PM, Jian Li <jianli at google.com> wrote:
> > Some parts of changes are due to the File API work I have worked on.
> > On Fri, Sep 3, 2010 at 2:50 PM, Adam Barth <abarth at webkit.org> wrote:
> >>
> >> I was looking at SecurityOrigin.cpp today and I saw a bunch of code
> >> relating to Blob URLs.  I don't really understand why this code is
> >> correct.  Would someone be willing to explain it to me?
> >>
> >> Some specific questions:
> >>
> >> 1) Why do blob URLs get exception from the unique origin check?  How
> >> does that interact with the HTML5 sandboxing model?
> >
> > The origin of blob URL is said to be the origin of the page under which
> the
> > blob URL is created. It is encoded as part of the blob URL:
> > blob:encoded_origin/id. We're not ignoring any security origin checks.
> > Instead, we need to pull the encoded origin out of the blob URL and use
> it
> > as the base for the origin check.
> >
> > The reason that we skip the unique origin check here is to allow a local
> > running worker script to be able to access a blob URL. Do we want to
> > disallow this case?
>
> The access rights of locally running content are controlled by a
> WebCore::Setting.  Currently, Chrome sets that setting to the most
> restrictive value to mitigate the harm a downloaded HTML file can
> cause.  It doesn't seem like a good idea to circumvent that security
> setting.
>

Ok, this sounds right. It means that we should not allow accessing a blob
URL from a local worker script per the settings, right?  If so, I can get
rid of this particular step.

>
> > If there is a security reason for doing this, I can go
> > ahead to revert this part of change.
>
> Thanks.
>
> >> 2) Why does SecurityOrigin::canLoad take a document as an argument?
> >> What are the semantics of this parameter?  In particular, why does a
> >> SecurityOrigin::canLoad ignore |this| when called with a document
> >> argument on a blob URL?  That seems like a very bad idea.
> >
> > SecurityOrigin::canLoad is a static method. Does it have |this| to use?
>
> Oh, that's right.  SecurityOrigin::canLoad is junk we moved over from
> FrameLoader.  We need to make it an instance method since all the
> callers have a document anyway.
>
> I don't quite understand what this code is trying to do:
>
> bool SecurityOrigin::canLoad(const KURL& url, const String& referrer,
> Document* document)
> {
> #if ENABLE(BLOB)
>    if (url.protocolIs("blob") && document) {
>        SecurityOrigin* documentOrigin = document->securityOrigin();
>        RefPtr<SecurityOrigin> targetOrigin = SecurityOrigin::create(url);
>        return documentOrigin->isSameSchemeHostPort(targetOrigin.get());
>    }
> #endif
>
> Why should canLoad care about isSameSchemeHostPort?  In the past,
> canLoad's job was to stop web sites from loading content from your
> local file system (e.g., in frames or as images).
>

Per the File API spec, the blob URL should not be accessed from the page in
other security domain. Checking isSameSchemeHostPort will help us enforce
this policy when other page in different domain that tries to load this URL
from the cache. Probably I need to put better comment here or maybe find a
better place to do such check.


>
> Adam
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20100903/75da83f7/attachment.html>