[webkit-dev] Blob changes to SecurityOrigin.cpp

[Adam Barth]

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.

> 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).

Adam