OS_LinuxBSD::shell_open does not unset LD_PRELOAD, which breaks Chrome w/ Steam on Linux

[darksylinc]

Godot version

4.2.x master [16a93563bfd3b02ca0a8f6df2026f3a3217f5571]

System information

Godot v4.2.dev (262d1eaa6) - Ubuntu 20.04.6 LTS (Focal Fossa) - X11 - Vulkan (Forward+) - dedicated AMD Radeon RX 6800 XT - AMD Ryzen 9 5900X 12-Core Processor (24 Threads)

Issue description

Steam injects Overlays.

It does so by using LD_PRELOAD.

Because the apps we open inherit our environment variables, trying to open Chrome with OS_LinuxBSD::shell_open, e.g. OS::get_singleton()->shell_open( "https:://www.google.com" ); will fail.

The reason for this failure is that Chrome performs integrity checks to detect whether it's being tampered.

Chrome detects Steam's Overlays injection as potential malware and thus refuses to launch.

For reference SDL2 had the same problem and resolved it with these two commits: https://github.com/libsdl-org/SDL/commit/9b7b928765ce732ca3024a42d0281fea7192ecaf & https://github.com/libsdl-org/SDL/pull/4567

Steps to reproduce

1. Open an URL with OS::get_singleton()->shell_open( "https:://www.google.com" );
2. Publish the project to Steam.
3. Ensure Steam Overlays are enabled on the client machine (usually on by default).
4. Ensure Chrome is the default browser that xdg-open will use.
5. Chrome will fail to load.

Minimal reproduction project

None.

[akien-mga]

We can do the same as SDL, but that seems very arbitrary. I'd argue that Steam is broken here, and they should figure out a less hacky way to enable the overlay on Steam games, and only those.

I can foresee issues with the "fix", e.g. if you make a game launcher using Godot. Or the Godot editor itself, where the project manager spawns a new process.

Or a Godot app that actually relies on LD_PRELOAD for something, e.g. running a compiled tool with bundled shared libraries.

I guess we can add the hack only for OS::shell_open and not for OS::execute, though that still feels inconsistent.

[darksylinc]

The fix should probably go into OS::execute though.

This sounds like it could be part of a setting. e.g. one that when enabled (default: enabled) problematic environment variables are stripped. Documentation would say something like "Don't allow child process to inherit problematic environment variables".

And the C++ code would be:

enum Behavior
{
    UseGlobalSetting,
    InheritProblematicEnvVariables,
    StripProblematicEnvVariables,
};

OS::shell_open( url, behavior = UseGlobalSetting );

OS::execute( process_name, behavior = UseGlobalSetting );

[rsubtil]

Can't this be worked around by doing this?

OS.unset_environment("LD_PRELOAD")
OS.shell_open("...")

[darksylinc]

Can't this be worked around by doing this?

OS.unset_environment("LD_PRELOAD")
OS.shell_open("...")

This is the core concept, yes.

However the devil in the details:

1. Users expect OS.shell_open("...") to simply work. They don't know about the LD_PRELOAD stuff on Linux + Steam.
2. Modifying environment variables is subject to possible race conditions, thus while calling OS.unset_environment("LD_PRELOAD"), if there are other threads reading environment variables, it could be problematic

[rsubtil]

1. Users expect OS.shell_open("...") to simply work. They don't know about the LD_PRELOAD stuff on Linux + Steam.

Right, but that's still a very specific combination of Linux + Steam + Chrome. Changing the default behavior of shell_open/execute to fix this scenario could then start creating problems on other use cases, for something that seemingly can be handled in GDScript.

2. Modifying environment variables is subject to possible race conditions, thus while calling OS.unset_environment("LD_PRELOAD"), if there are other threads reading environment variables, it could be problematic

If this were to be implemented in code it would be subject to the same problem. I think we should wrap internal set_environment/unset_environment calls in a global mutex then.

Also on another note, does Steam offer any API to open URLs? All I could find is ISteamFriends.ActivateGameOverlayToWebPage, not sure if there's an official API they offer for this.

[AThousandShips]

For internal thread safety in the engine these methods are not guaranteed to be thread safe so no thread safety stuff should be added here

[darksylinc]

Right, but that's still a very specific combination of Linux + Steam + Chrome.

That's the most common combination. Most users default to Chrome, and most Linux users will run from Steam.

Even the Editor runs into this problem when clicking on URL links to online documentation.

Changing the default behavior of shell_open/execute to fix this scenario could then start creating problems on other use cases, for something that seemingly can be handled in GDScript.

Like I said, this can be fixed by exposing the behavior to the user.

Opening a bundled tool can be fixed by setting InheritProblematicEnvVariables, assuming the tool breaks when using LD_PRELOAD.

LD_PRELOAD is used on the following situations:

1. To inject stuff (like Steam Overlays, custom Vulkan / OpenGL)
2. To fix compatibility problems (e.g. the app depends on a specific version of libC)

The most likely case is that Godot-specific subprocesses will need this env variable, while anything external (e.g. anything that requires opening with xdg-open, or opening anything in /usr or ~/.local) should not inherit LD_PRELOAD.

[darksylinc]

Removed comment because I linked to an unrelated ticket.

[rsubtil]

Alright, so I've been thinking about both shell_open and execute, and considering their likely users and use-cases:

• As you suggested, this behavior should be controllable in both methods. While trivial to handle in GDScript, having it be part of the method call and documentation brings much more attention to this scenario, which can also be further described in documentation. This can also be implemented with a default argument and not break compatibility.
• For shell_open I now agree with changing it to clear LD_PRELOAD by default, because it's more likely to see users hitting this issue and not having a clear idea of what's happening. When I mentioned the "specific combination of Linux + Steam + Chrome" I failed to notice this LD_PRELOAD mechanism is not relevant on Windows/macOS, so I agree that on Linux this combination is likely common. It's not really Godot's fault and is still a hack to account for it, but I think changing it would do more good than harm.
• For execute, out of precaution, I'd keep the current behavior of keeping LD_PRELOAD as is. There could be use-cases which rely on this behavior, and I'm not seeing a strong argument to change this for execute. For example, I've tested running the Godot editor on Renderdoc, then launching the project. Configured properly, Renderdoc can hook on child processes and propagate properly. If this behavior were to be changed, this would stop working, without the user being able to control it. I'm not sure if this is a regular workflow for some, but if it is, we would break it unnecessarily, while not bringing any advantages (from what I've searched so far).

A few more notes as well:

• I've tested clearing the LD_PRELOAD when opening links on a Steam Deck, and it works without issue.

• There could be more problematic env vars to strip, as noted on a similar discussion for SnapCraft

  Even the Editor runs into this problem when clicking on URL links to online documentation.

• Even though this should fix that, it actually seems that this should not be happening at all, since Steam disables injecting the overlay for Software apps. Might be worth to investigate this further..?