Looks like the IPC message validation is being too strict about valid URL formats coming from the renderer. Hush, didn't we have a problem like this in the past that we already fixed? I can't find the bug now but vaguely remember something.

I have never seen this IPC failure before in fact.
Bad message reason 95 is RDH_ILLEGAL_ORIGIN

Bo any ideas? I see you dealt with similar issues with LoadDataWithBaseURL before

I'm guessing something to do the fact the base url isn't a valid url

I logged here:
https://cs.chromium.org/chromium/src/content/browser/loader/resource_dispatcher_host_impl.cc?rcl=0&l=319

And canCommitURL is false, IsIllegalOrigin is also false (which means the origin "foobar://" is considered a legal origin)

IsIllegalOrigin is irrelevant, webview doesn't override that method

in fact this has to do with the HTML content itself, not with the origin, because if I change the htmlData to some simple HTML then no crash.

it must be trying to navigate then, if it's something relative to the base url/host, then that's probably going to blow up, since base url is not a valid url

<style type="text/css"> @font-face {font-family: 'Lato';font-style: normal;font-weight: 400;src: local('Lato Regular'), local('Lato-Regular'), url('http://fonts.gstatic.com/s/lato/v11/v0SdcGFAl2aezM9Vq_aFTQ.ttf') format('truetype');}

There is this line in the html that wants to download the ttf file. And that is the request that failed the security checks, because somehow, Chromium thinks that the origin of this font request is foobar:// instead of fonts.gstatic.com/

There are other thinkgeek.com requests and they are all fine.

First question is why does the font request header contain an origin of "foobar://"?
Second question is should we crash in this scenario instead of just silently fail. After all you're not supposed to crash on bad sites.

creis@ any ideas here?

Actually I misremembered, "foobar://" is a valid gurl apparently ("http://" is not)

Anyway, my thought is when we do loadData(baseUrl=foobar://, ...), it probably should grant the foobar:// scheme to the renderer from the ChildProcessSecurityPolicyImpl::GrantRequestURL call?

Comment 10: The Origin header is a CORS thing, attached to XHR requests, font requests, and certain other data types.  It's used to let the server know the origin of the page making the request, so the server can decide whether to allow it or not.  See https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS for details.

In this case, the origin of the page making the request is foobar:// (as absurd as that is), so the header is correct.

The logic in ShouldServiceRequest is checking the following: "Check if the renderer is using an illegal Origin header.  If so, kill it."  This is necessary to prevent a compromised renderer process from claiming to be something it's not allowed to be, like a Chrome App, WebUI page, or (eventually) a violation of Site Isolation.

Here, ChildProcessSecurityPolicy doesn't know that the renderer is allowed to commit a strange scheme like foobar://.  I think I agree with boliu@ that granting access to the origin when a LoadDataWithBaseURL call is made makes sense.  Maybe use GrantOrigin, though?

> I think I agree with boliu@ that granting access to the origin when a LoadDataWithBaseURL call is made makes sense.  Maybe use GrantOrigin, though?

I actually thought //content already calls GrantRequestURL for LoadDataWithBaseURL, but maybe something more subtle broke with the weird base url. Sounds like you are suggesting it's just not called at all?

GrantRequestURL is called for a normal LoadURL though, right?

Yes, it looks like GrantRequestURL should be called via RFH::Navigate.  You should check to see why that isn't getting to the GrantScheme call, or why CanCommitURL isn't finding the scheme.

First blind guess: GrantRequestURL is called with the wrong url..

Webview only call GrantRequestURL with the data url.

guessed correctly, should call it with the base url instead

For GrantScheme: webview only calls GrantScheme with file scheme and content scheme during webview start up sequence. 

https://cs.chromium.org/chromium/src/content/browser/frame_host/render_frame_host_impl.cc?rcl=0&l=2827

Right now we have a logic that only GrantRequestURL with the base url if the base url scheme is file scheme. Looks like we have to do so regardless of the base url's scheme.

I'm not sure about the security implications though. 
+clamy who added this call.

CL uploaded here https://codereview.chromium.org/2165783003

Hello Charlie,
One more question:
There is a behavior difference between single process and multi process on Android.
https://cs.chromium.org/chromium/src/content/public/browser/browser_message_filter.cc?rcl=1469531834&l=162

In single process mode, if we receive BrowserMessageFilter::ShutdownForBadMessage for any reason, we don't shut down the child process because there is no child process. --- This seems to be a security problem.
In multi process mode, we shut down the renderer and crash.

on Linux, renderer will crash in both single process and multi process.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4bdb4f88f8197bebe32ce3480a3851201f648a8b

commit 4bdb4f88f8197bebe32ce3480a3851201f648a8b
Author: hush <hush@chromium.org>
Date: Tue Jul 26 18:56:30 2016

Grant permission to the base url when loadDataWithBaseURL is called.

When there's a base URL specified for the data URL, we also need to
grant access to the base URL. This allows file: and other unexpected
schemes to be accepted at commit time and during CORS checks (e.g., for
font requests).

BUG= 627564 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2165783003
Cr-Commit-Position: refs/heads/master@{#407867}

[modify] https://crrev.com/4bdb4f88f8197bebe32ce3480a3851201f648a8b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/4bdb4f88f8197bebe32ce3480a3851201f648a8b/content/browser/frame_host/render_frame_host_impl.cc

hush@: Is this fixed after r407867?

yes, I think so