Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insecure use of /tmp in libappindicator-based SDL_Tray implementation #11887

Closed
smcv opened this issue Jan 8, 2025 · 0 comments · Fixed by #11888
Closed

Insecure use of /tmp in libappindicator-based SDL_Tray implementation #11887

smcv opened this issue Jan 8, 2025 · 0 comments · Fixed by #11888

Comments

@smcv
Copy link
Contributor

smcv commented Jan 8, 2025

src/tray/unix/SDL_tray.c uses a predictable path in /tmp, which makes it vulnerable to a classic time-of-check/time-of-use vulnerability involving symbolic links. On a multi-user system, an attacker who can predict (or learn) the process ID of a SDL game that uses the tray could predict the filename and create a symbolic link /tmp/sdl_appindicator_icon_${pid}_${sequence}.bmp.

When SDL writes out a new icon with that name, it will traverse the symbolic link and overwrite a filename of the attacker's choice using the privileges of the victim. There is an impact on availability (denial of service) by making the victim overwrite a file that they wanted to keep, and possibly also an impact on confidentiality or integrity (for instance if the attacker overwrites a file that is meant to be kept secret as part of an authentication mechanism).

For example:

victim$ ls ~/will-be-overwritten
(doesn't exist, or has "old" contents)
victim$ ${builddir}/test/testtray

attacker$ cd /tmp
attacker$ ls sdl_*
sdl_appindicator_icon_169458_0.bmp
sdl_appindicator_icon_169458_1.bmp
attacker$ ln -s /home/victim/will-be-overwritten sdl_appindicator_icon_169458_2.bmp

(victim uses the "Change Icon" menu option)

victim$ ls ~/will-be-overwritten
(exists, has been overwritten with a copy of the icon)

Recent Linux kernels optionally mitigate this with the fs.protected_symlinks sysctl, which many distros enable by default. Other Unix OSs (*BSD, etc.) might have similar OS-specific mitigations.

The 3.1.8 preview release and git snapshots > 3.1.6 (specifically since 01b9b0e, I think) are vulnerable. SDL 2.x and the 3.1.6 preview release are unaffected.

I think the simplest solution to this vulnerability is to create a temporary directory owned by the user who is running the SDL program, with restrictive permissions (0700), and put the .bmp files in there: other users will be unable to modify that directory, so we are protected. This does not protect against an attacker who is able to run arbitrary non-sandboxed code under the victim's uid, but under the typical Unix security model nothing can be protected against such an attacker anyway.

libappindicator already depends on GLib and GTK, so we can use GLib's g_mkdtemp() or g_dir_make_tmp() rather than needing to wait for #10966 or similar.

smcv added a commit to smcv/SDL that referenced this issue Jan 8, 2025
If we write directly to filenames in /tmp, we're subject to
time-of-check/time-of-use symlink attacks on most systems (although
recent Linux kernels mitigate these by default). We can avoid these
attacks by securely creating a directory owned by our own uid,
and doing all our file I/O in that directory. Other uids cannot create
symbolic links in that directory, so we are protected from symlink
attacks.

This does not protect us from an attacker that is running with the same
uid, but if such an attacker exists, then we have already lost.

Resolves: libsdl-org#11887
Signed-off-by: Simon McVittie <[email protected]>
smcv added a commit to smcv/SDL that referenced this issue Jan 8, 2025
If we write directly to filenames in /tmp, we're subject to
time-of-check/time-of-use symlink attacks on most systems (although
recent Linux kernels mitigate these by default). We can avoid these
attacks by securely creating a directory owned by our own uid,
and doing all our file I/O in that directory. Other uids cannot create
symbolic links in that directory, so we are protected from symlink
attacks.

This does not protect us from an attacker that is running with the same
uid, but if such an attacker exists, then we have already lost.

Resolves: libsdl-org#11887
Signed-off-by: Simon McVittie <[email protected]>
smcv added a commit to smcv/SDL that referenced this issue Jan 8, 2025
If we write directly to filenames in /tmp, we're subject to
time-of-check/time-of-use symlink attacks on most systems (although
recent Linux kernels mitigate these by default). We can avoid these
attacks by securely creating a directory owned by our own uid,
and doing all our file I/O in that directory. Other uids cannot create
symbolic links in that directory, so we are protected from symlink
attacks.

This does not protect us from an attacker that is running with the same
uid, but if such an attacker exists, then we have already lost.

Resolves: libsdl-org#11887
Signed-off-by: Simon McVittie <[email protected]>
@slouken slouken closed this as completed in ef1fdf1 Jan 8, 2025
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 a pull request may close this issue.

1 participant