xorl %eax, %eax

NILFSv2 Utilities system() Privilege Escalation

with 2 comments

This bug was reported by Eric Sandeen on 11 June 2009. As you can see from Jussi Lehtola’s comment mkfs.nilfs2, nilfs_cleanerd, mount.nilfs2 and umount.nilfs2 are all installed as SUID root binaries. This was patched by updating the equivalent Makefiles which you can see here and here. In addition to this, user could use mkfs.nilfs2 with “-c” option which would lead to this code:

int main(int argc, char *argv[])
{
	struct nilfs_disk_info diskinfo, *di = &diskinfo;
	struct mkfs_options *opts = &options;
	struct nilfs_segment_info *si;
	const char *device;
	int fd;

	parse_options(argc, argv, opts);
	device = argv[optind];

	blocksize = opts->blocksize;

	if (opts->cflag)
		disk_scan(device, opts);  /* check the block device */
      ...
	close(fd);
	exit(0);
}

If user utilizes the “-c” flag, then disk_scan() will be invoked passing the user controlled device and the rest of the options. A quick look at this routine from sbin/mkfs/mkfs.c reveals that…

/*
 * I/O routines & primitives
 */
static void disk_scan(const char *device, struct mkfs_options *opts)
{
	char buf[COMMAND_BUFFER_SIZE];
	ssize_t n;

	n = snprintf(buf, COMMAND_BUFFER_SIZE, "badblocks -b %ld %s %s %s\n",
		     opts->blocksize, opts->quiet ? "" : "-s",
		     (opts->cflag > 1) ? "-w" : "", device);
	if (n < 0 || n >= COMMAND_BUFFER_SIZE)
		perr("Internal error: command line buffer overflow");
	if (!opts->quiet)
		pinfo("checking blocks");
	system(buf);
}

The user can execute almost anything he wants since this is a SUID root application and it calls system() with completely user controlled arguments as you can see in the above routine. To fix this, disk_scan() was completely rewritten to use execv() instead of system like this:

 static void disk_scan(const char *device, struct mkfs_options *opts)
 {
-       char buf[COMMAND_BUFFER_SIZE];
-       ssize_t n;
+       struct stat statbuf;
+       pid_t pid;
+       int status;
+
+       if (stat(badblocks, &statbuf) != 0) {
+               pinfo("Warning: %s not found.", badblocks);
+               return;
+       }
 
-       n = snprintf(buf, COMMAND_BUFFER_SIZE, "badblocks -b %ld %s %s %s\n",
-                    opts->blocksize, opts->quiet ? "" : "-s",
-                    (opts->cflag > 1) ? "-w" : "", device);
-       if (n < 0 || n >= COMMAND_BUFFER_SIZE)
-               perr("Internal error: command line buffer overflow");
        if (!opts->quiet)
                pinfo("checking blocks");
-       system(buf);
+
+       pid = fork();
+       if (pid == 0) {
+               char bszbuf[BLOCKSIZE_BUFFER_SIZE];
+               const char *args[7];
+               ssize_t n;
+               int i = 0;
+
+               if (setgid(getgid()) < 0)
+                       perr("Error: failed to drop setgid privileges");
+               if (setuid(getuid()) < 0)
+                       perr("Error: failed to drop setuid privileges");
+               args&#91;i++&#93; = badblocks;
+               args&#91;i++&#93; = "-b";
+               n = snprintf(bszbuf, BLOCKSIZE_BUFFER_SIZE, "%ld",
+                            opts->blocksize);
+               if (n < 0 || n >= BLOCKSIZE_BUFFER_SIZE)
+                       perr("Internal error: blocksize buffer overflow");
+               args[i++] = bszbuf;
+
+               if (!opts->quiet)
+                       args[i++] = "-s";
+               if (opts->cflag > 1)
+                       args[i++] = "-w";
+               args[i++] = device;
+               args[i] = NULL;
+               execv(badblocks, (char **)args);
+               exit(1); /* reach only if failed */
+       } else if (pid != -1) {
+               if (wait(&status) < 0)
+                       perr("Error: cannot wait child");
+               else if (!WIFEXITED(status))
+                       perr("Error: check aborted");
+               else if (WEXITSTATUS(status))
+                       perr("Error: check failed with status(%d)",
+                            WEXITSTATUS(status));
+       } else {
+               perr("Error: cannot fork: %s", strerror(errno));
+       }
 }
 
 static void check_mount(int fd, const char *device)
&#91;/sourcecode&#93;

In addition to this, a new check was added to main() using stat() to check that the given "device" is a block device file exactly before calling disk_scan() like this:

&#91;sourcecode language="c"&#93;
        blocksize = opts->blocksize;
 
+       if (stat(device, &statbuf) != 0)
+               perr("Error: cannot find %s: %s", device, strerror(errno));
+       else if (!S_ISREG(statbuf.st_mode) && !S_ISBLK(statbuf.st_mode))
+               perr("Error: device must be a block device or a file");
+
        if (opts->cflag)

Quite rare to see system() bugs in SUID root applications nowadays though… Anyway, the bug affects releases prior to 2.0.14.

Written by xorl

July 24, 2009 at 22:58

Posted in bugs, linux

2 Responses

Subscribe to comments with RSS.

  1. It also seems to be vulnerable to the old $PATH trick too as it calls ‘badblocks’ not using its full path.

    Boooboooo

    July 27, 2009 at 22:00

  2. @Boooboooo: It used to be before the patch, now “badblocks” is defined like:

    #define BADBLOCKS_NAME         "badblocks"
    static const char badblocks[] = "/sbin/" BADBLOCKS_NAME;
    

    at sbin/mkfs/mkfs.c.

    xorl

    July 28, 2009 at 11:47


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s