Closed Bug 1509384 Opened 1 year ago Closed 1 year ago

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

Categories

(Core :: DOM: Drag & Drop, defect, P2, critical)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- fixed

People

(Reporter: mats, Assigned: arai)

References

Details

(Keywords: assertion)

Attachments

(1 file, 1 obsolete file)

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.
:arai, do you know soemthing about this, given you worked on 1424107?
Flags: needinfo?(arai.unmht)
Priority: -- → P2
I'll look into this hopefully this weekend.
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.
Flags: needinfo?(arai.unmht)
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.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Attachment #9029388 - Flags: review?(enndeakin)
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.
Attachment #9029388 - Flags: review?(enndeakin)
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 on attachment 9029388 [details] [diff] [review]
Handle null-principal in drag-and-drop across processes.

submitting to phabricator
Attachment #9029388 - Attachment is obsolete: true
Attachment #9029416 - Attachment description: Bug 1509384 - Handle null-principal in drag-and-drop across processes. r?smaug → Bug 1509384 - Use IPC::Principal instead of Principal URI string in Drag-and-Drop. r?smaug
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/autoland/rev/32f94b7a8db6
Use IPC::Principal instead of Principal URI string in Drag-and-Drop. r=smaug
https://hg.mozilla.org/mozilla-central/rev/32f94b7a8db6
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Regressions: 1544120
You need to log in before you can comment on or make changes to this bug.