Calling WKPageLoadAlternateHTMLString() where baseURL does not point to a directory causes an invalid message to be returned from WebProcess

• Component: WebKit2
• Status: RESOLVED FIXED

[Ada Chan]

Calling WKPageLoadAlternateHTMLString() where baseURL does not point to a directory causes an invalid message to be returned from WebProcess, which then terminates the WebProcess.

In WebPageProxy::loadAlternateHTMLString(), it calls WebProcessProxy::assumeReadAccessToBaseURL() which adds the baseURL argument to the list of local directories that have read access (m_localPathsWithAssumedReadAccess). But if that URL actually points to a file, we just add its file path to that list without removing its last path component. So any subresource that's in the same folder as that file will fail the URL check in WebProcessProxy::checkURLReceivedFromWebProcess() during load because it does not reside in any directories specified in m_localPathsWithAssumedReadAccess. If the URL check fails, the message ends up being marked as invalid, causing the WebProcess to terminate.

One possible fix is in WebProcessProxy::assumeReadAccessToBaseURL(), we can get the base URL of the passed in URL, and then add its path to m_localPathsWithAssumedReadAccess. If the passed in URL is a directory, the behavior remains the same. If the URL points to a file though, we'll add the path to the containing directory to m_localPathsWithAssumedReadAccess instead.

[Ada Chan]

<rdar://problem/10289392>

[Alexey Proskuryakov]

> If the passed in URL is a directory, the behavior remains the same. If the URL points to a file though, we'll add the path to the containing directory to m_localPathsWithAssumedReadAccess instead.

That would match the behavior of HTML <base> tag, and thus makes good sense to me.

[Ada Chan]

> In WebPageProxy::loadAlternateHTMLString(), it calls WebProcessProxy::assumeReadAccessToBaseURL() which adds the baseURL argument to the list of local directories that have read access 
I meant the *urlString* argument.

[Ada Chan]

Created attachment 111126 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 111126 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=111126&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

The patch cannot be landed with this line.

> Source/WebCore/ChangeLog:10
> +        * WebCore.exp.in:

Please describe what's changed.

[Ada Chan]

(In reply to comment #5)
> (From update of attachment 111126 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=111126&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        No new tests. (OOPS!)
> 
> The patch cannot be landed with this line.

Fixed.

> 
> > Source/WebCore/ChangeLog:10
> > +        * WebCore.exp.in:
> 
> Please describe what's changed.

Fixed.

Thanks for reviewing!

Patch has been landed: http://trac.webkit.org/changeset/97633