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

SDL_filesystem: get temporary folder #10841

Open
madebr opened this issue Sep 14, 2024 · 13 comments · May be fixed by #10966
Open

SDL_filesystem: get temporary folder #10841

madebr opened this issue Sep 14, 2024 · 13 comments · May be fixed by #10966
Milestone

Comments

@madebr
Copy link
Contributor

madebr commented Sep 14, 2024

Would it make sense to get the system-dependent temporary folder using the SDL_filesystem api?
Or do we then also need to think about the bigger picture, and provide SDL alternatives of mkstemp and family?

These folder categories currently exist:

typedef enum SDL_Folder
{
SDL_FOLDER_HOME, /**< The folder which contains all of the current user's data, preferences, and documents. It usually contains most of the other folders. If a requested folder does not exist, the home folder can be considered a safe fallback to store a user's documents. */
SDL_FOLDER_DESKTOP, /**< The folder of files that are displayed on the desktop. Note that the existence of a desktop folder does not guarantee that the system does show icons on its desktop; certain GNU/Linux distros with a graphical environment may not have desktop icons. */
SDL_FOLDER_DOCUMENTS, /**< User document files, possibly application-specific. This is a good place to save a user's projects. */
SDL_FOLDER_DOWNLOADS, /**< Standard folder for user files downloaded from the internet. */
SDL_FOLDER_MUSIC, /**< Music files that can be played using a standard music player (mp3, ogg...). */
SDL_FOLDER_PICTURES, /**< Image files that can be displayed using a standard viewer (png, jpg...). */
SDL_FOLDER_PUBLICSHARE, /**< Files that are meant to be shared with other users on the same computer. */
SDL_FOLDER_SAVEDGAMES, /**< Save files for games. */
SDL_FOLDER_SCREENSHOTS, /**< Application screenshots. */
SDL_FOLDER_TEMPLATES, /**< Template files to be used when the user requests the desktop environment to create a new file in a certain folder, such as "New Text File.txt". Any file in the Templates folder can be used as a starting point for a new file. */
SDL_FOLDER_VIDEOS, /**< Video files that can be played using a standard video player (mp4, webm...). */
SDL_FOLDER_COUNT /**< Total number of types in this enum, not a folder type by itself. */
} SDL_Folder;

@Semphriss
Copy link
Contributor

I planned to keep SDL_Folder and SDL_GetUserFolder for folders that are managed by the user and whose path depend on the users' preferences and other settings; I describe it with more detail in #9538 (comment), #8051 and #7665 (comment).

In short: I wanted to "do one thing and do it well". The function is meant to return folders that are under the user's control like "My Stuff", and I preferred to keep folders that would need further OS- or app-dependant treatment (like mktemp, the Trash folder, AppData or the OS readonly font folder) for different functions. My linked comments go more in detail.

I'd support adding other OS-specific folders, but I would encourage writing their own functions for them, since each of them would need some special functionality and may require additional parameters (Trash folder needs to remember the files' original locations, mktemp has code to generate a safe temp folder, etc).


Assuming we decide to stick to my vision of things, @slouken should I rename the SDL_Folder enum to SDL_UserFolder?

@madebr
Copy link
Contributor Author

madebr commented Sep 14, 2024

My current use case for getting a temporary path is for testing redirection of process stdin/stdout/stderr to/from a file.

@Semphriss
Copy link
Contributor

Should an eventual SDL_CreateTempFile/Folder return a path or an IOStream object? The latter would reduce the chances of timing attacks in cases where the contents of the temp file are sensitive.

@slouken
Copy link
Collaborator

slouken commented Sep 19, 2024

It should return an IOStream object, though we should probably add a file path property.

@Semphriss
Copy link
Contributor

I believe I read that some systems can create temporary files that don't have path equivalents, only file descriptors, but now I can't find those functions anymore so I'm not sure if it's relevant to mention them.

@slouken
Copy link
Collaborator

slouken commented Sep 19, 2024

I believe I read that some systems can create temporary files that don't have path equivalents, only file descriptors, but now I can't find those functions anymore so I'm not sure if it's relevant to mention them.

I think you're thinking of mkstemp:
https://man7.org/linux/man-pages/man3/mkstemp.3.html

@Semphriss
Copy link
Contributor

Semphriss commented Sep 19, 2024

mkstemp creates a file that has a path equivalent, which is formatted according to its only argument (most commonly something like /tmp/tmp.XXXXXX). I'm thinking of a function that creates a file that does not have any path, e. g. cannot be accessed by any path.

@slouken
Copy link
Collaborator

slouken commented Sep 19, 2024

Ah, gotcha.

@Semphriss
Copy link
Contributor

After some more research, it appears that tmpfile most closely approaches what I think I saw, except that the docs directly mention the filename. I distinctly remember one where it was an extra security feature that the file didn't have a name at all, and was accessible only through its file descriptor.

No matter what terms I search for, I cannot find anything about such a function. If I trust my usual luck, I will find nothing, start thinking I've hallucinated that function, and give up, and the moment the API is settled on beyond any further modification, I'll finally find it.

@NekkoDroid
Copy link

You might be thinking of memfd_create https://man7.org/linux/man-pages/man2/memfd_create.2.html

@v1993
Copy link

v1993 commented Sep 21, 2024

There's also O_TMPFILE flag for open on Linux: https://man7.org/linux/man-pages/man2/open.2.html; a major difference from memfd_create is that the resulting descriptor is backed by file system rather than memory. It's used by tmpfile on Linux under the hood, see: https://github.com/bminor/glibc/blob/751a5502bea1d13551c62c47bb9bd25bff870cda/stdio-common/tmpfile.c#L46 and https://github.com/bminor/glibc/blob/751a5502bea1d13551c62c47bb9bd25bff870cda/sysdeps/unix/sysv/linux/gentempfd.c#L27-L28.

@Semphriss Semphriss linked a pull request Sep 27, 2024 that will close this issue
@slouken slouken added this to the 3.x milestone Oct 6, 2024
@smcv
Copy link
Contributor

smcv commented Jan 8, 2025

Should an eventual SDL_CreateTempFile/Folder return a path or an IOStream object? The latter would reduce the chances of timing attacks in cases where the contents of the temp file are sensitive.

At least on Unix, the problem with using paths for temporary files is usually not "other processes might read its contents". If the attacker is the same uid and non-sandboxed, then you've already lost because they can read your memory contents via debugging interfaces, and if the attacker is a different uid, permissions are enough to protect you (any reasonable temporary file API will either default to 600/700 permissions or require the mode as a parameter, for this reason).

Instead, the classic problem with using paths for temporary files is "the attacker might trick you into overwriting some unrelated file". See also #11887, https://en.wikipedia.org/wiki/Symlink_race and https://capec.mitre.org/data/definitions/27.html. https://security.opensuse.org/2023/12/14/budgie-extras-predictable-tmp-paths.html is a typical real-world example.

(I don't know what the security model is on Windows, things might be different there.)

@smcv
Copy link
Contributor

smcv commented Jan 8, 2025

I'm thinking of a function that creates a file that does not have any path, e. g. cannot be accessed by any path.

In the absence of system-specific facilities like Linux memfds and O_TMPFILE, the closest you can get to this is to create a file and then unlink it, leaving the file descriptor in your process as the only way to access that file. This is how tmpfile() traditionally worked, and presumably how it still works on non-Linux. If the file is in a directory that is writeable by other users (like /tmp) then it needs to be done just as carefully and atomically as for path-based interfaces, to avoid the classic symlink attack.

The ability to unlink a file while keeping it opened is a POSIX thing, and I'm not sure it's possible on Windows.

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.

6 participants