pidgin/pidgin

[gaim-migrate @ 16061]

2006-04-19, Mark Doliner
3ade3ab0b2ae
Parents 13d0d6abc1d3
Children cffcc9a3cf9e
[gaim-migrate @ 16061]
silcgaim_check_silc_dir() checks to make sure the user's private
key has permission 0600. If it doesn't, it chmod's the file.
Nathanael Hoyle pointed out the totally absurd scenario where, if
Gaim is suid root, someone could replace the private key with
something else between the fstat and the chmod so that the file
permissions are changed on a file that the user wouldn't otherwise
have access to. He also suggested a fix along the lines of this
one.

Ethan said this still isn't totally safe, but it should be a little
better, and I don't really care anyway because you'd have to be a
moron to run Gaim with the suid bit set in the first place.
1 files changed, 16 insertions(+), 2 deletions(-)
  • +16 -2
    src/protocols/silc/util.c
    • --- a/src/protocols/silc/util.c Tue Apr 18 21:16:45 2006 +0000
    +++ b/src/protocols/silc/util.c Wed Apr 19 02:12:45 2006 +0000
    @@ -75,6 +75,7 @@
    char pkd[256], prd[256];
    struct stat st;
    struct passwd *pw;
    + int fd;
    pw = getpwuid(getuid());
    if (!pw) {
    @@ -225,6 +226,7 @@
    }
    #endif
    + fd = open(file_private_key, O_RDONLY);
    if ((g_stat(file_private_key, &st)) == -1) {
    /* If file doesn't exist */
    if (errno == ENOENT) {
    @@ -234,10 +236,15 @@
    file_public_key, file_private_key, NULL,
    (gc->password == NULL) ? "" : gc->password,
    NULL, NULL, NULL, FALSE);
    + if (fd != -1)
    + close(fd);
    + fd = open(file_private_key, O_RDONLY);
    g_stat(file_private_key, &st);
    } else {
    gaim_debug_error("silc", "Couldn't stat '%s' private key, error: %s\n",
    file_private_key, strerror(errno));
    + if (fd != -1)
    + close(fd);
    return FALSE;
    }
    }
    @@ -246,23 +253,30 @@
    /* Check the owner of the private key */
    if (st.st_uid != 0 && st.st_uid != pw->pw_uid) {
    gaim_debug_error("silc", "You don't seem to own your private key!?\n");
    + if (fd != -1)
    + close(fd);
    return FALSE;
    }
    /* Check the permissions for the private key */
    if ((st.st_mode & 0777) != 0600) {
    gaim_debug_warning("silc", "Wrong permissions in your private key file `%s'!\n"
    - "Trying to change them ... ", file_private_key);
    - if ((chmod(file_private_key, 0600)) == -1) {
    + "Trying to change them ...\n", file_private_key);
    + if ((fd != -1) && (fchmod(fd, S_IRUSR | S_IWUSR)) == -1) {
    gaim_debug_error("silc",
    "Failed to change permissions for private key file!\n"
    "Permissions for your private key file must be 0600.\n");
    + if (fd != -1)
    + close(fd);
    return FALSE;
    }
    gaim_debug_warning("silc", "Done.\n\n");
    }
    #endif
    + if (fd != -1)
    + close(fd);
    +
    return TRUE;
    }