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

Doom: m_config.c should use the user's home directory for config and save games #2

Open
bcoles opened this issue Mar 30, 2020 · 1 comment

Comments

@bcoles
Copy link

bcoles commented Mar 30, 2020

By default, SerenityDOOM uses the current working directory . to store and load configuration files and save game files.

config.c :

// Get the path to the default configuration dir to use, if NULL
// is passed to M_SetConfigDir.

static char *GetDefaultConfigDir(void)
{
    char *result = (char *)malloc(2);
    result[0] = '.';
    result[1] = '\0';

    return result;
}

The M_GetSaveGameDir function attempts to create a directory by concatenating configdir and "savegame/", resulting in a hidden directory called .savegame which probably wasn't intended. The intended path was probably meant to be ./savegame/.

//
// Calculate the path to the directory to use to store save games.
// Creates the directory as necessary.
//

char *M_GetSaveGameDir(char *iwadname)
{
    char *savegamedir;
#if ORIGCODE
    char *topdir;
#endif

# ...

        savegamedir = M_StringJoin(configdir, "savegame/", NULL);

        M_MakeDirectory(savegamedir);

        printf ("Using %s for savegames\n", savegamedir);

Creation of directories in the current working directory creates the possibility for symlink attacks.

Should root desire to slay some demons while browsing a directory writable by low privileged users, such as /tmp, and should a low privileged user have preemptively created the following directory structure, then /etc/passwd would be overwritten by root's saved game.

$ mkdir /tmp/.savegame
$ ln -s /etc/passwd /tmp/.savegame/temp.dsg
@bcoles
Copy link
Author

bcoles commented Nov 1, 2020

This has been patched upstream ozkl#1

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

No branches or pull requests

1 participant