1509384 - Fatal assertion when dragging text from data: URL (Assertion failure: !StringBeginsWith(aOrigin, static_cast<const nsLiteralCString&>(nsLiteralCString("" "moz-nullprincipal" ":"))) (CreateCodebasePrincipal does not support NullPrincipal)

Status: RESOLVED FIXED

(Core :: DOM: Copy & Paste and Drag & Drop, defect, P2)

Description

[Mats Palmgren (inactive)]

STR
1. load data:text/html,xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx in a DEBUG build
2. select some text on the page
3. drag that text selection

ACTUAL RESULT
Assertion failure: !StringBeginsWith(aOrigin, static_cast<const nsLiteralCString&>(nsLiteralCString("" "moz-nullprincipal" ":"))) (CreateCodebasePrincipal does not support NullPrincipal), at caps/BasePrincipal.cpp:459

Fwiw, dragging text from a https page seems to work fine.

Comment 1

[Neil Deakin]

:arai, do you know soemthing about this, given you worked on 1424107?

Comment 2

[Tooru Fujisawa [:arai]]

I'll look into this hopefully this weekend.

Comment 3

[Tooru Fujisawa [:arai]]

The URL comes from here:
  https://searchfox.org/mozilla-central/rev/8f0db72fb6e35414fb9a6fc88af19c69f332425f/widget/nsDragServiceProxy.cpp#68

then, looks like we can use nsScriptSecurityManager::CreateCodebasePrincipalFromOrigin to filter out such case:
  https://searchfox.org/mozilla-central/rev/8f0db72fb6e35414fb9a6fc88af19c69f332425f/caps/nsScriptSecurityManager.cpp#1239-1254
I think the assertion corresponds to the branch there

nsScriptSecurityManager::CreateCodebasePrincipalFromOrigin fails for system principal, extended principal, and null principal.
maybe just fallback to null-principal in that case?
so that such website cannot try to open "file:///" or something.

Comment 4

[Tooru Fujisawa [:arai]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b057a03c5e884ab2a5dd7adf1fd5cfe17b84f75e&group_state=expanded

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Handle null-principal in drag-and-drop across processes.

The problem is that if the dragged item's principal is null, the mDragPrincipalURISpec is set to its URL.
(while it's set to null for system principal or extended principal)

We should detect that case and use null-principal.
Then, nsIScriptSecurityManager::CreateCodebasePrincipalFromOrigin has that logic.
so I replaced BasePrincipal::CreateCodebasePrincipal with it.
and also put system principal case into else-branch given all other cases are handled in then-branch.

Comment 6

[Neil Deakin]

Comment on attachment 9029388 [details] [diff] [review]
Handle null-principal in drag-and-drop across processes.

I don't think I understand the security implications of this patch to review it, sorry.

Comment 7

[Tooru Fujisawa [:arai]]

About the current behavior, we fall into the following branch, and it creates null-principal anyway, with default attribute.

https://searchfox.org/mozilla-central/rev/cfaa5a1d48d6bc6552199e73004ecb05d0a9c921/caps/BasePrincipal.cpp#360-364
> already_AddRefed<BasePrincipal> BasePrincipal::CreateCodebasePrincipal(
>     nsIURI* aURI, const OriginAttributes& aAttrs) {
>   MOZ_ASSERT(aURI);
> 
>   nsAutoCString originNoSuffix;
>   nsresult rv =
>       ContentPrincipal::GenerateOriginNoSuffixFromURI(aURI, originNoSuffix);
>   if (NS_FAILED(rv)) {
>     // If the generation of the origin fails, we still want to have a valid
>     // principal. Better to return a null principal here.
>     return NullPrincipal::Create(aAttrs);
>   }

The new behavior does the same thing in the earlier branch.
So the behavior doesn't change.

Comment 8

[Tooru Fujisawa [:arai]]

Comment on attachment 9029388 [details] [diff] [review]
Handle null-principal in drag-and-drop across processes.

submitting to phabricator

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Bug 1509384 - Use IPC::Principal instead of Principal URI string in Drag-and-Drop. r?smaug

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/32f94b7a8db6
Use IPC::Principal instead of Principal URI string in Drag-and-Drop. r=smaug

Comment 11

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/32f94b7a8db6