1926884 - Long profile path can break the Mac remote detection

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Tooru Fujisawa [:arai]]

Steps to reproduce:

1. Build with ./mach build on macOS
2. Launch firefox with OBJDIR/dist/Nightly.app/Contents/MacOS/firefox -foreground -profile /tmp/long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long
3. Hit "cmd+option+shift+i" (the Browser Toolbox shortcut)

Actual result:
The Browser Toolbox window is opened in the same process, and it shows error.

Browser Toolbox connection status:

Error: Must pass a port in an env variable with MOZ_BROWSER_TOOLBOX_PORT
connect@chrome://devtools/content/framework/browser-toolbox/window.js:99:11
@chrome://devtools/content/framework/browser-toolbox/window.js:173:13

Expected result:
The Browser Toolbox window is opened in the dedicate process.

This seems to be coming from the fact that the message port's name has some length limitation, as the message port's name contains the profile path, and the issue happens when the profile path is very long.

The Browser Toolbox's dedicate profile is created inside the main profile, which means, the prefix of those path are common.
And it would imply that when only the common prefix is treated as the message port name, those 2 are mixed up.

The limit could be possibly abound 128, given the message port forms Mozilla_firefox-default_PATHHERE_RemoteWindow style, and the issue starts when the common prefix between the main profile and the Browser Toolbox profile exceeds 128 characters or so.

https://searchfox.org/mozilla-central/rev/dca2603d55b5b39d3b8ab8e93c08b42563f5aad8/toolkit/components/remote/nsMacRemoteClient.mm#26-30

nsString className;
BuildClassName(aProgram, aProfile, className);
NSString* serverNameString = [NSString
    stringWithCharacters:reinterpret_cast<const unichar*>(className.get())
                  length:className.Length()];

https://searchfox.org/mozilla-central/rev/dca2603d55b5b39d3b8ab8e93c08b42563f5aad8/toolkit/components/remote/RemoteUtils.h#15-17,24

static void BuildClassName(const char* aProgram, const char* aProfile,
                           nsString& aClassName) {
  aClassName.AppendPrintf("Mozilla_%s", aProgram);
...
  aClassName.AppendPrintf("_%s_RemoteWindow", aProfile);

marking as security-sensitive just in case.

Comment 1

[Tooru Fujisawa [:arai]]

This would be the underlying issue of bug 1926803.

Mossop, can I have your input here?
Do you think this can cause any security issue beyond the Browser Toolbox failure and some case where firefox command getting forwarded to unexpected existing process?

Comment 2

[Dave Townsend [:mossop]]

(In reply to Tooru Fujisawa [:arai] from comment #1)

This would be the underlying issue of bug 1926803.

Ah, good catch. We could probably work around this by using some simple hash of the profile path but we'd have to consider how to roll that out safely.

Mossop, can I have your input here?
Do you think this can cause any security issue beyond the Browser Toolbox failure and some case where firefox command getting forwarded to unexpected existing process?

I can't think of one. It requires being able to execute a process with certain command line arguments and at that point you already have to have escaped any sandboxing and could just call Firefox without a specific profile path directly anyway.

Comment 3

[Tooru Fujisawa [:arai]]

Thank you for your input!

I also confirmed a similar limitation is applied on Windows, where the limit is 256 chars, and if the profile path is too long, firefox fails to start because of RegisterClassW call fails.

I'll apply a length limitation to each component of the class name, with adding a hash of the full string.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Bug 1926884 - Use short name if the remoting name hits the OS limitation. r?mossop!

Comment 5

[Tooru Fujisawa [:arai]]

The problem on Windows is just that the process crashes during startup in nsWinRemoteServer::Startup, and I expect this not to be security sensitive either.
So, I think we can open this bug up?

Comment 6

[Dave Townsend [:mossop]]

(In reply to Tooru Fujisawa [:arai] from comment #5)

The problem on Windows is just that the process crashes during startup in nsWinRemoteServer::Startup, and I expect this not to be security sensitive either.
So, I think we can open this bug up?

I agree.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c145f42a113
Use short name if the remoting name hits the OS limitation. r=mossop

Comment 8

[amarc]

https://hg.mozilla.org/mozilla-central/rev/4c145f42a113