Skip to content

Fix #103: Resolve local privilege escalation in API IPC path#104

Closed
NodeAndNails wants to merge 4 commits into
AceSLS:mainfrom
NodeAndNails:main
Closed

Fix #103: Resolve local privilege escalation in API IPC path#104
NodeAndNails wants to merge 4 commits into
AceSLS:mainfrom
NodeAndNails:main

Conversation

@NodeAndNails
Copy link
Copy Markdown

Dropping the API IPC file in /tmp (world-writable) let any local user write "install|appId|library" to it, forcing Steam to install arbitrary apps.

  • Moved the IPC file to $XDG_RUNTIME_DIR with 0600 permissions.
  • Falls back to ~/.config/SLSsteam if XDG isn't set.
  • Added lstat() checks on the fallback dir to block symlink traversal and verified ownership to stop cross-UID attacks.
  • Checked mkdir() return value so we don't silently run in a broken state if it fails.
  • Added a stat()/geteuid() check in onFileChange() to reject commands if the file owner doesn't match (TOCTOU mitigation).
  • Added SLSAPI::deinit() to clean up the IPC file.
  • Wired deinit() into unload() in main.cpp. (Note: Steam's normal exit skips unload(), so XDG tmpfs cleanup on logout is the actual safety net).
  • Changed path to std::string in api.hpp so we can handle dynamic paths without buffer issues.

Dropping the API IPC file in /tmp (world-writable) let any local user write "install|appId|library" to it, forcing Steam to install arbitrary apps.

- Moved the IPC file to $XDG_RUNTIME_DIR with 0600 permissions.
- Falls back to ~/.config/SLSsteam if XDG isn't set.
- Added lstat() checks on the fallback dir to block symlink traversal and verified ownership to stop cross-UID attacks.
- Checked mkdir() return value so we don't silently run in a broken state if it fails.
- Added a stat()/geteuid() check in onFileChange() to reject commands if the file owner doesn't match (TOCTOU mitigation).
- Added SLSAPI::deinit() to clean up the IPC file.
- Wired deinit() into unload() in main.cpp. (Note: Steam's normal exit skips unload(), so XDG tmpfs cleanup on logout is the actual safety net).
- Changed path to std::string in api.hpp so we can handle dynamic paths without buffer issues.
@JoacoSlime
Copy link
Copy Markdown

This PR is getting stalled more than it should.
Is there any problems, or why is it getting delayed?

@NodeAndNails
Copy link
Copy Markdown
Author

Closing this in favor of #109. Sorry, I forgot to make a separate branch initially and pushed straight to main. The new PR has the exact same fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants