1851788 - Only accept content browsing context ids for BiDi commands when "system access" is not enabled

Status: NEW

(Remote Protocol :: WebDriver BiDi, defect, P2)

Description

[Julian Descottes [:jdescottes]]

STRs:

• try to send a navigate or reload command to a chrome browsing context id (eg "1" or "2")

ER: Should fail with noSuchFrame
AR: Throws the following error:

 {
  "type": "error",
  "id": 6,
  "error": "unknown error",
  "message": "TypeError: can't access property \"browsingContext\", webProgress is null",
  "stacktrace": "#awaitNavigation@chrome://remote/content/webdriver-bidi/modules/root/browsingContext.sys.mjs:1043:21\nreload@chrome://remote/content/webdriver-bidi/modules/root/browsingContext.sys.mjs:935:32\nhandleCommand@chrome://remote/content/shared/messagehandler/MessageHandler.sys.mjs:255:33\nexecute@chrome://remote/content/shared/webdriver/Session.sys.mjs:274:32\nonPacket@chrome://remote/content/webdriver-bidi/WebDriverBiDiConnection.sys.mjs:211:37\nonMessage@chrome://remote/content/server/WebSocketTransport.sys.mjs:127:18\nhandleEvent@chrome://remote/content/server/WebSocketTransport.sys.mjs:109:14\n"
}

Comment 1

[Julian Descottes [:jdescottes]]

As discussed in the meeting, any command which allows to send a specific context id might pass the BrowsingContext id of a chrome context. By default, chrome Browsing Contexts have quite predictable ids: "1", "2", "3"...

Our context ids are a combination of UUIDs generated by tab manager (for top level contexts) and actual platform BrowsingContext ids, which means that we will fallback to BrowsingContext.get when the user provides a context id. And in that case we will start trying to apply the command to this chrome context.

After testing, it doesn't seem that any existing command can effectively interact with those contexts.
Commands which need to use the child actor can't work because our actors are only created for messageManagerGroups: ["browsers"], and we also throw in any case around https://searchfox.org/mozilla-central/rev/0d51e137a45a3657b39be3c27d4c7271fdb11d59/remote/shared/messagehandler/transports/RootTransport.sys.mjs#114

So only fully parent process commands could actually try to run against chrome contexts. If I listed them right

• browser.close: does not support a context argument
• browsingContext.close: no effect
• browsingContext.navigate: fails when trying to access the webprogress
• browsingContext.reload: fails when trying to access the webprogress
• browsingContext.print: fails with 0x80070057 (NS_ERROR_ILLEGAL_VALUE)
• browsingContext.setViewport: fails with TypeError: can't access property \"style\", browser is null

Nevertheless, we should make sure the browsing contexts we retrieve are content contexts until we support chrome scope.
Or maybe we should always create UUIDs, even for non-toplevel browsing contexts.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Thanks for testing Julian. Given that none of the commands allow escaping from content and run code that is not wanted I feel that we could potentially make this a confidential bug only?

Comment 3

[Julian Descottes [:jdescottes]]

I agree we can remove the security flag but we are not able to do it ourselves.

Freddy: can you or someone from the security group remove the security flag?

Comment 4

[Frederik Braun [:freddy] (OOO until August 18th)]

Done. Do you also want the "confidential Mozilla employee bug" removed?

Comment 5

[Julian Descottes [:jdescottes]]

Thanks!

Do you also want the "confidential Mozilla employee bug" removed?

No this is fine, we want to keep this internal until it is fixed.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:whimboo, could you have a look please?

For more information, please visit BugBot documentation.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Slightly rewording the bug's summary to avoid overloading the context identifier by adopting the term scope for consistency.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

I think that we can remove the confidential flag from this bug given that it will be a feature that is publicly usable.