1459562 - Opening a blob URL image in a new window makes firefox to crash

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Andrea Marchesini [:baku]]

STR:

1. A simple page creating a blob URL, containing an image, and displaying that URL using a <img> element.
2. right click on the image.
3. click on "view image", keeping 'SHIFT' key pressed.

Expected results:
a new window must appear, showing the blob URL image.

Actual results:
a new process is spawn and it crashes immediately saying:

[Child 15762, Main Thread] WARNING: This content process hasn't received the permissions for http://localhost:8000 yet: file /home/baku/Sources/m/blob/src/extensions/cookie/nsPermissionManager.cpp, line 3406
Assertion failure: PermissionAvailable(prin, aType), at /home/baku/Sources/m/blob/src/extensions/cookie/nsPermissionManager.cpp:2342

Comment 1

[Andrea Marchesini [:baku]]

Nika, you have worked on permissions. Could it be related? Do you mind to investigate?

Comment 2

[Andrea Marchesini [:baku]]

Attachment: test.zip

Here the file I used to reproduce this issue.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

(In reply to Andrea Marchesini [:baku] from comment #1)
> Nika, you have worked on permissions. Could it be related? Do you mind to
> investigate?

Hmm, it sounds like permissions aren't sent down with blob requests? I presume that a blob URI has a principal associated with it. When we send the data into a content process, we'll need to TransmitPermissionsForPrincipal for that principal, e.g. https://searchfox.org/mozilla-central/rev/a85db9e29eb3f022dbaf8b9a6390ecbacf51e7dd/dom/ipc/ContentParent.cpp#5363-5364.

I'm guessing that the load doesn't go through that AboutToLoad codepath, which is why the permissions aren't being transmitted right now.

Comment 4

[Andrea Marchesini [:baku]]

Well, this is not what happens when we load blob URIs. In this case, the channel doesn't run on the parent process:

https://searchfox.org/mozilla-central/rev/00dd116638001772fe354b081353b73f1cad405d/dom/file/nsHostObjectProtocolHandler.cpp#904-907,962-967

nsHostObjectProtocolHandler::NewChannel2 uses NS_NewInputStreamChannelInternal and this channel doesn't interact with the parent process at all.

We should transmit the permissions when the blobURLs are broadcasted.

I'm not particularly happy about blobURLs broadcasting, but, the solution I have in mind would require a big refactoring of how principal::CheckMayLoad() works, and the removal of nsIURIWithPrincipal. This is not going to happen soon.

Comment 5

[Andrea Marchesini [:baku]]

Attachment: blobURLs.patch

Here a fix.

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8976068 [details] [diff] [review]
blobURLs.patch

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

::: dom/ipc/ContentParent.cpp
@@ +2452,5 @@
>      }
> +
> +    for (const BlobURLRegistrationData& registration : registrations) {
> +      rv = TransmitPermissionsForPrincipal(registration.principal());
> +      Unused << NS_WARN_IF(NS_FAILED(rv));

Please do this only if you're actually calling SendInitBlobURLs. Also, please do it first.

It might be a good idea to add a bulk transmitting option to TransmitPermissionsForPrincipal, but we can look into that later if this shows up as an issue.

@@ +5140,5 @@
>        }
>  
>        Unused << cp->SendBlobURLRegistration(uri, ipcBlob, principal);
> +
> +      rv = cp->TransmitPermissionsForPrincipal(principal);

Please transmit the permissions before you send the registration. I think that this right now will cause a race condition if something tries to perform a load after the registration is added, but before the permissions are transmitted.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ea614928f8b
Transmit the permissions of the owning principal together with the blobURL when broadcasted to content processes, r=nika

Comment 8

[Narcis Beleuzu [:NarcisB]]

Backed out for build bustages on ContentParent.cpp
For Linux, the bustage was : "..../nsError.h:34:77: error: cannot convert 'mozilla::DebugOnly<nsresult>' to 'nsresult' for argument '1' to 'uint32_t NS_FAILED_impl(nsresult)'" 

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/786865568ed76134a3f9724956949e5d48f34210
Push link: https://hg.mozilla.org/integration/mozilla-inbound/rev/1ea614928f8b0f959aa09ef4674118d27d1e317e
Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=178952087&repo=mozilla-inbound&lineNumber=24915
Linux log link: https://treeherder.mozilla.org/logviewer.html#?job_id=178952087&repo=mozilla-inbound&lineNumber=24915

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2908cc9102b0
Transmit the permissions of the owning principal together with the blobURL when broadcasted to content processes, r=nika

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/2908cc9102b0

Comment 11

[Anca Soncutean, Desktop QA]

I haven’t managed to reproduce this issue using the test case from comment 2 and the steps from bug’s description (using localhost:8000) on Ubuntu 16.04 (both x64 and x32 bits) and Windows 10 x64 with Firefox 61.0a1 (2018-05-07). For me, no crash or error occurs, the blob URL image is displayed successfully in a new window. Is maybe something else that I missed or misinterpreted?

Comment 12

[Andrea Marchesini [:baku]]

This happens when the new window is loads the blobURL on a new process. That assertion is triggered only on debug builds.

Comment 13

[Anca Soncutean, Desktop QA]

Thank you for your prompt reply!

I’ve manage to reproduce the crash with Firefox 61.0a1 (20180507085552) debug build under Ubuntu 16.04 x64. 
This bug is fixed on debug build 62.0b9 (20180713154019) under Ubuntu 16.04 x64, Windows 10 x64 and macOS 10.13.