From: hpa Date: Mon, 10 Feb 2003 10:51:41 +0000 (+0000) Subject: Change the installer to use mtools instead of trying to play games X-Git-Tag: syslinux-3.11~613 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5a5e30b45acbdbe799cd4460ffef50294c538a06;p=platform%2Fupstream%2Fsyslinux.git Change the installer to use mtools instead of trying to play games with mounting the filesystem. --- diff --git a/NEWS b/NEWS index f57b8c9..107d8dd 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,14 @@ Starting with 1.47, changes marked with SYSLINUX/PXELINUX/ISOLINUX apply to that specific program only; other changes apply to all of them. +Changes in 2.02: + * SYSLINUX: Security flaws have been found in the SYSLINUX + installer when running setuid root. Rewrite the SYSLINUX + installer so it uses mtools instead. It therefore now + requires mtools (specifically mcopy and mattrib) to exist on + your system, but it will not require root privileges and + SHOULD NOT be setuid. + Changes in 2.01: * MEMDISK: Fix memory sizing bug when the ramdisk crosses the 16 MB boundary. diff --git a/syslinux.c b/syslinux.c index 0138965..5f7c4eb 100644 --- a/syslinux.c +++ b/syslinux.c @@ -14,14 +14,9 @@ /* * syslinux.c - Linux installer program for SYSLINUX * - * This program ought to be portable. I hope so, at least. - * - * HPA note: this program needs too much privilege. We should probably - * access the filesystem directly like mtools does so we don't have to - * mount the disk. Either that or if Linux gets an fmount() system call - * we probably could do the mounting ourselves, and make this program - * setuid safe. Or perhaps try to link with mtools code... - * + * This program now requires mtools. It turned out to be a lot + * easier to deal with than dealing with needing mount privileges. + * We need device write permission anyway. */ #define _XOPEN_SOURCE 500 /* Required on glibc 2.x */ @@ -43,18 +38,8 @@ #include "syslinux.h" -#ifndef _PATH_MOUNT -#define _PATH_MOUNT "/bin/mount" -#endif - -#ifndef _PATH_UMOUNT -#define _PATH_UMOUNT "/bin/umount" -#endif - char *program; /* Name of program */ char *device; /* Device to install to */ -uid_t ruid; /* Real uid */ -uid_t euid; /* Initial euid */ pid_t mypid; enum bs_offsets { @@ -182,30 +167,19 @@ ssize_t xpwrite(int fd, void *buf, size_t count, off_t offset) int main(int argc, char *argv[]) { static unsigned char sectbuf[512]; - int dev_fd, fd; + int dev_fd; struct stat st; int veryold; unsigned int sectors, clusters; - int err = 0; - pid_t f, w; int status; - char *mntpath = NULL, mntname[128], devfdname[128]; - char *ldlinux_name, **argp, *opt; - mode_t my_umask; + char **argp, *opt; int force = 0; /* -f (force) option */ off_t offset = 0; /* -o (offset) option */ - static const char * const clean_environ[] = { - "PATH=/bin:/usr/bin", - NULL - }; + char mtools_conf[] = "/tmp/syslinux-mtools-XXXXXX"; + int mtc_fd; + FILE *mtc, *mtp; - ruid = getuid(); - euid = geteuid(); mypid = getpid(); - - if ( !euid ) - seteuid(ruid); /* Run as regular user until we need it */ - program = argv[0]; device = NULL; @@ -258,15 +232,7 @@ int main(int argc, char *argv[]) exit(1); } - if ( lseek(dev_fd, offset, SEEK_SET) != offset ) { - if ( !(force && errno == EBADF) ) { - fprintf(stderr, "%s: seek error", device); - exit(1); - } - } - xpread(dev_fd, sectbuf, 512, offset); - fsync(dev_fd); /* * Check to see that what we got was indeed an MS-DOS boot sector/superblock @@ -325,205 +291,51 @@ int main(int argc, char *argv[]) } /* - * Now mount the device. If we are non-root we need to find an fstab - * entry for this device which has the user flag and the appropriate - * options set. + * Create an mtools configuration file */ - if ( euid ) { - FILE *fstab; - struct mntent *mnt; - - if ( !(fstab = setmntent(MNTTAB, "r")) ) { - fprintf(stderr, "%s: cannot open " MNTTAB "\n", program); - } - - while ( (mnt = getmntent(fstab)) ) { - if ( !strcmp(device, mnt->mnt_fsname) && - ( !strcmp(mnt->mnt_type, "msdos") || - !strcmp(mnt->mnt_type, "umsdos") || - !strcmp(mnt->mnt_type, "vfat") || - !strcmp(mnt->mnt_type, "uvfat") || - !strcmp(mnt->mnt_type, "auto") ) && - ( hasmntopt(mnt, "user") || - (hasmntopt(mnt, "owner") && st.st_uid == euid) ) && - !hasmntopt(mnt, "ro") && - mnt->mnt_dir[0] == '/' && - !!hasmntopt(mnt, "loop") == !!S_ISREG(st.st_mode) && - ( (!hasmntopt(mnt,"offset") && offset == 0) || - (atol(hasmntopt(mnt, "offset")) == offset) ) ) { - /* Okay, this is an fstab entry we should be able to live with. */ - - mntpath = mnt->mnt_dir; - break; - } - } - endmntent(fstab); - - if ( !mntpath ) { - fprintf(stderr, "%s: not root and no appropriate entry for %s in " - MNTTAB "\n", program, device); - exit(1); - } - - f = fork(); - if ( f < 0 ) { - perror(program); - exit(1); - } else if ( f == 0 ) { - /* We're not root here, so don't use clean_environ */ - execl(_PATH_MOUNT, _PATH_MOUNT, mntpath, NULL); - _exit(255); /* If execl failed, trouble... */ - } - } else { - unsigned int nn; - struct stat dst; - int rv; - - /* We're root or at least setuid. - Make a temp dir and pass all the gunky options to mount. */ - - if ( chdir("/tmp") ) { - perror(program); - exit(1); - } - -#define TMP_MODE (S_IXUSR|S_IWUSR|S_IXGRP|S_IWGRP|S_IWOTH|S_IXOTH|S_ISVTX) - - if ( stat(".", &dst) || !S_ISDIR(dst.st_mode) || - (dst.st_mode & TMP_MODE) != TMP_MODE ) { - fprintf(stderr, "%s: possibly unsafe /tmp permissions\n", program); - exit(1); - } - - for ( nn = 1 ; nn ; nn++ ) { - snprintf(mntname, sizeof mntname, "syslinux.mnt.%lu.%u", - (unsigned long)mypid, nn); - - if ( lstat(mntname, &dst) != -1 || errno != ENOENT ) - continue; - - seteuid(0); /* *** BECOME ROOT *** */ - rv = mkdir(mntname, 0000); /* AS ROOT */ - seteuid(ruid); - - if ( rv == -1 ) { - if ( errno == EEXIST || errno == EINTR ) - continue; - perror(program); - exit(1); - } - - if ( lstat(mntname, &dst) || dst.st_mode != (S_IFDIR|0000) || - dst.st_uid != 0 ) { - fprintf(stderr, "%s: someone is trying to symlink race us!\n", program); - openlog("syslinux", LOG_PID, LOG_AUTHPRIV); - syslog(LOG_WARNING, - "symlink race intercepted when running as user %lu", - (unsigned long)ruid); - exit(1); - } - break; /* OK, got something... */ - } - - if ( !nn ) { - fprintf(stderr, "%s: failed to create a temp directory!\n", program); - exit(1); - } - - mntpath = mntname; - - snprintf(devfdname, sizeof devfdname, "/proc/%lu/fd/%d", - (unsigned long)mypid, dev_fd); - - f = fork(); - if ( f < 0 ) { - perror(program); - rmdir(mntpath); - exit(1); - } else if ( f == 0 ) { - char mnt_opts[128]; - seteuid(0); /* ***BECOME ROOT*** */ - setuid(0); - if ( S_ISREG(st.st_mode) ) { - snprintf(mnt_opts, sizeof mnt_opts, - "rw,nodev,noexec,nosuid,loop,offset=%" PRIdMAX ",umask=077,uid=%lu", - (uintmax_t)offset, (unsigned long)ruid); - } else { - snprintf(mnt_opts, sizeof mnt_opts, - "rw,nodev,noexec,nosuid,umask=077,uid=%lu", - (unsigned long)ruid); - } - /* We're root, use clean_environ */ - execle(_PATH_MOUNT, _PATH_MOUNT, "-t", "msdos", "-o", mnt_opts,\ - devfdname, mntpath, NULL, clean_environ); - _exit(255); /* execl failed */ - } - } - - w = waitpid(f, &status, 0); - if ( w != f || (!WIFEXITED(status) || WEXITSTATUS(status)) ) { - if ( !euid ) { - seteuid(0); /* *** BECOME ROOT *** */ - rmdir(mntpath); /* AS ROOT */ - seteuid(ruid); - } - exit(1); /* Mount failed */ - } - - ldlinux_name = alloca(strlen(mntpath)+13); - if ( !ldlinux_name ) { + mtc_fd = mkstemp(mtools_conf); + if ( mtc_fd < 0 || !(mtc = fdopen(mtc_fd, "w")) ) { perror(program); - err = 1; - goto umount; - } - sprintf(ldlinux_name, "%s/ldlinux.sys", mntpath); - - unlink(ldlinux_name); - fd = open(ldlinux_name, O_WRONLY|O_CREAT|O_TRUNC, 0444); - if ( fd < 0 ) { - perror(device); - err = 1; - goto umount; + exit(1); } - - xpwrite(fd, syslinux_ldlinux, syslinux_ldlinux_len, 0); - + fprintf(mtc, + "drive s:\n" + " file=\"/proc/%lu/fd/%d\"\n" + " offset=%lld\n", + (unsigned long)mypid, + dev_fd, + (unsigned long long)offset); + fclose(mtc); + /* - * I don't understand why I need this. Does the DOS filesystems - * not honour the mode passed to open()? + * Run mtools to create the LDLINUX.SYS file */ - my_umask = umask(0777); - umask(my_umask); - fchmod(fd, 0444 & ~my_umask); - - close(fd); - -umount: - f = fork(); - if ( f < 0 ) { - perror("fork"); + if ( setenv("MTOOLSRC", mtools_conf, 1) ) { + perror(program); exit(1); - } else if ( f == 0 ) { - seteuid(0); /* ***BECOME ROOT*** */ - setuid(0); - execle(_PATH_UMOUNT, _PATH_UMOUNT, mntpath, NULL, clean_environ); } - w = waitpid(f, &status, 0); - if ( w != f || (!WIFEXITED(status) || WEXITSTATUS(status)) ) { + /* This command may fail legitimately */ + system("mattrib -h -r -s s:ldlinux.sys 2>/dev/null"); + + mtp = popen("mcopy -o - s:ldlinux.sys", "w"); + if ( !mtp || + (fwrite(syslinux_ldlinux, 1, syslinux_ldlinux_len, mtp) + != syslinux_ldlinux_len) || + (status = pclose(mtp), !WIFEXITED(status) || WEXITSTATUS(status)) ) { + fprintf(stderr, "%s: failed to create ldlinux.sys\n", program); exit(1); } - sync(); + status = system("mattrib +r s:ldlinux.sys"); - if ( !euid ) { - seteuid(0); /* *** BECOME ROOT *** */ - rmdir(mntpath); /* AS ROOT */ - seteuid(ruid); + if ( !WIFEXITED(status) || WEXITSTATUS(status) ) { + fprintf(stderr, + "%s: warning: failed to set readonly bit on ldlinux.sys\n", + program); } - if ( err ) - exit(err); + unlink(mtools_conf); /* * To finish up, write the boot sector