[MPlayer-dev-eng] [PATCH] minor security fix to fibmap_mplayer

[Adam Rice]

fibmap_mplayer as distributed opens the supplied filename as root. This can be
used by an attacker to:

a) Open devices in /dev. This can cause kernel modules to load and tapes to
rewind. In some cases in may be possible for an attacker to crash the system
by triggering module loading bugs such as race conditions.

b) Test the existance of files the attacker wouldn't normally be able to
access. This is not a major security hole in itself, but it can be used to
gather data as part of an attack, and of course it's a privacy violation.
Example: determine if root has a .bashrc file:
 > fibmap_mplayer /root/.bashrc
 
The attached patch fixes these problems by dropping root privileges except for
the FIBMAP ioctl itself. I've also tried to avoid writing any output with root
privileges, as this has been associated with attacks on special files in /proc
in the past.

The patch applies cleanly against MPlayer-20031121 and MPlayer-1.0pre2. I've
only tested it on Linux, I don't know if FIBMAP is even used on other
systems.

Adam Rice

-- 
Adam Rice -- adamrice at ntlworld.com -- Blackburn, Lancashire, England
-------------- next part --------------
--- fibmap_mplayer.c.old	2003-11-21 20:07:17.000000000 +0000
+++ fibmap_mplayer.c	2003-11-21 20:59:11.000000000 +0000
@@ -17,22 +17,43 @@
 
 int main ( int argc , char ** argv )
 {
-	int fd,lba=0;
-	if (argc!=2) {
-	    fprintf(stderr,"Bad usage.\n");
+	int fd,ret,lba=0;
+	if (geteuid()!=0) {
+	    fprintf(stderr, "%s must be setuid root to work\n",
+	    argv[0]);
+                       return 1;
+	}
+	if (seteuid(getuid()) == -1) {
+	    fprintf(stderr, "Couldn't drop privileges: %s\n",
+	    strerror(errno));
+	    return 1;
+	}
+	if (argc!=2 || argv[1]==NULL) {
+	    fprintf(stderr,"Usage: %s <filename>\n", argv[0]);
 	    return 1;
 	}
 	if ((fd = open(argv[1], O_RDONLY)) == -1) {
     	    fprintf(stderr,"Cannot open file %s: %s\n",
-	    argv[1] ? argv[1] : "(NULL)", strerror(errno));
+	    argv[1], strerror(errno));
     	    return 1;
 	}
-        if (ioctl(fd, FIBMAP, &lba) != 0) {
-	    fprintf(stderr,"fibmap ioctl: %s (Hint: %s is not suid root?)\n",strerror(errno),argv[0]);
-            close(fd);
+        if (seteuid(0) == -1) {
+            fprintf(stderr, "Couldn't restore root privileges: %s\n",
+            strerror(errno));
+            return 1;
+        }
+        ret = ioctl(fd, FIBMAP, &lba);
+        if (seteuid(getuid()) == -1) {
+            fprintf(stderr, "Couldn't re-drop privileges: %s\n",
+            strerror(errno));
+            return 1;
+        }
+        close(fd);
+        if (ret != 0) {
+	    fprintf(stderr,"fibmap ioctl failed: %s\n",
+	    strerror(errno));
             return 1;
         }
-	close(fd);
 	printf("%d\n",lba);
 	return 0;
 }