From bfa76a8553f0a824115aba58f8615ca3da40a788 Mon Sep 17 00:00:00 2001 From: Enrico Scholz Date: Thu, 2 Oct 2003 15:35:57 +0000 Subject: [PATCH] rewrote it completely git-svn-id: http://svn.linux-vserver.org/svn/util-vserver/trunk@48 94cd875c-1c1d-0410-91d2-eb244daf1a30 --- util-vserver/src/secure-mount.c | 177 +++++++++++++++++++++++----------------- 1 file changed, 102 insertions(+), 75 deletions(-) diff --git a/util-vserver/src/secure-mount.c b/util-vserver/src/secure-mount.c index c61111e..83bab05 100644 --- a/util-vserver/src/secure-mount.c +++ b/util-vserver/src/secure-mount.c @@ -16,18 +16,11 @@ // Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. - // secure-mount + // secure-mount // - // mounts into the chroot at destination in a way + // mounts into the chroot at destination in a way // that prevents symlink attacks - // algorithm: - // enter chroot - // mount tmpfs-directory - // leave chroot - // mount --bind tmpfs - // enter chroot - // mount --move tmpfs #ifdef HAVE_CONFIG_H # include @@ -37,16 +30,17 @@ #define VERSION_COPYRIGHT_AUTHOR "Enrico Scholz" #include "util.h" -#include -#include -#include -#include -#include #include -#include +#include #include #include - +#include +#include +#include +#include +#include +#include +#include #define MNTPOINT "/etc" @@ -56,7 +50,10 @@ showHelp(int fd, char const *cmd, int res) WRITE_MSG(fd, "Usage: "); WRITE_STR(fd, cmd); WRITE_MSG(fd, - " \n\n" + " \n\n" + "Mounts to / and assumes \n" + "as trustworthy but as hostile. Therefore, will be\n" + "traversed in a secure way and must not contain symlinks.\n\n" "Please report bugs to " PACKAGE_BUGREPORT "\n"); exit(res); @@ -66,20 +63,90 @@ static void showVersion() { WRITE_MSG(1, - "secure-mount " VERSION " -- secure mounting of directories in chroots\n" + "secure-mount " VERSION " -- secure mounting of directories\n" "This program is part of " PACKAGE_STRING "\n" "Copyright (C) 2003 Enrico Scholz\n" VERSION_COPYRIGHT_DISCLAIMER); exit(0); } -int main(int argc, char *argv[]) +inline static bool +isSameObject(struct stat const *lhs, + struct stat const *rhs) +{ + return (lhs->st_dev==rhs->st_dev && + lhs->st_ino==rhs->st_ino); +} + +static int +chdirSecure(char const *dir) +{ + char tmp[strlen(dir)+1], *ptr; + char const *cur; + + strcpy(tmp, dir); + cur = strtok_r(tmp, "/", &ptr); + while (cur) { + struct stat pre_stat, post_stat; + + if (lstat(cur, &pre_stat)==-1) return -1; + + if (!S_ISDIR(pre_stat.st_mode)) { + errno = ENOENT; + return -1; + } + if (S_ISLNK(pre_stat.st_mode)) { + errno = EINVAL; + return -1; + } + + if (chdir(cur)==-1) return -1; + if (stat(".", &post_stat)==-1) return -1; + + if (!isSameObject(&pre_stat, &post_stat)) { + char dir[PATH_MAX]; + + WRITE_MSG(2, "Possible symlink race ATTACK at '"); + WRITE_STR(2, getcwd(dir, sizeof(dir))); + WRITE_MSG(2, "'\n"); + + errno = EINVAL; + return -1; + } + + cur = strtok_r(0, "/", &ptr); + } + + return 0; +} + +static int +verifyPosition(char const *mntpoint, char const *dir1, char const *dir2) { - int fd = open("/", O_RDONLY); - // HACK: sanity checks should be done before declaration, but this does - // not work with C89 compilers - char path[strlen(argc==1 ? "" : argv[1]) + sizeof(MNTPOINT) + 3]; + struct stat pre_stat, post_stat; + + if (stat(mntpoint, &pre_stat)==-1) return -1; + if (chroot(dir1)==-1 || chdir(dir2)==-1) return -1; + if (stat(".", &post_stat)==-1) return -1; + + if (!isSameObject(&pre_stat, &post_stat)) { + char dir[PATH_MAX]; + + WRITE_MSG(2, "Possible symlink race ATTACK at '"); + WRITE_STR(2, getcwd(dir, sizeof(dir))); + WRITE_MSG(2, "' within '"); + WRITE_MSG(2, dir1); + WRITE_STR(2, "'\n"); + + errno = EINVAL; + return -1; + } + return 0; +} + +int main(int argc, char *argv[]) +{ if (argc>=2) { if (!strcmp(argv[1], "--help")) showHelp(1, argv[0], 0); if (!strcmp(argv[1], "--version")) showVersion(); @@ -89,66 +156,26 @@ int main(int argc, char *argv[]) exit(255); } - if (chroot(argv[1]) == -1 || - chdir("/") == -1) { - perror("chroot()/chdir()"); - exit(1); - } - - if (mount("none", MNTPOINT, "tmpfs", 0, "size=4k")==-1) { - perror("mount()"); - exit(1); + if (chdir(argv[1])==-1) { + perror("chdir()"); + return 1; } - if (mkdir(MNTPOINT "/x",0600) == -1) { - perror("mkdir()/fchdir()"); - goto err1; - } - - if (fchdir(fd) == -1 || - chroot(".") == -1) { - perror("chroot()"); - goto err2; + if (chdirSecure(argv[3])==-1) { + perror("chdirSecure()"); + return 1; } - strcpy(path, argv[1]); - strcat(path, MNTPOINT "/x"); - - if (mount(argv[2], path, "", MS_BIND, 0)==-1) { + if (mount(argv[2], ".", "", MS_BIND, 0)==-1) { perror("mount()"); - goto err3; - } - - if (chroot(argv[1]) == -1) { - perror("chroot()"); - goto err4; - } - - if (chdir("/") == -1 || - mount(MNTPOINT "/x", argv[3], "", MS_MOVE, 0) == -1) { - perror("chdir()/mount()"); - goto err5; - } - - if (umount(MNTPOINT) == -1) { - perror("umount()"); + return 1; } - return 0; - - err5: - err4: - if (umount(path)==-1) perror("umount(path)"); - - err3: - if (chdir(argv[1])==-1) { - perror("chdir()"); + // Check if directories were moved between the chdirSecure() and mount(2) + if (verifyPosition(argv[2], argv[1], argv[3])==-1) { + // TODO: what is with unmounting? return 1; } - err2: - err1: - if (umount("mnt")==-1) perror("umount(\"mnt\")"); - - return 1; + return 0; } -- 1.8.1.5