Re: [CFR] diskpart(1) buffer overflow fix

[Peter Pentchev]

On Mon, Dec 02, 2002 at 01:58:09PM +0200, Peter Pentchev wrote:
> Hi,
> 
> As noted on the vuln-dev list recently, the diskpart(1) program in
> -stable is susceptible to a buffer overflow in the parsing of
> command-line arguments.  This is a low-risk problem, since diskpart(1)
> is not - and has never been, and has no reason to ever be - a privileged
> program, but still, there should be no harm in fixing it :)
> 
> Attached are two patches: a trivial one which just fixes up two problems
> in diskpart's argument parsing, and a more complex one, which does it
> "the right way" IMHO, using getopt(3).
> 
> Comments?

And a comment from myself: of course it would have been way better if I
had actually attached the patches...

G'luck,
Peter

-- 
Peter Pentchev  [EMAIL PROTECTED]        [EMAIL PROTECTED]
PGP key:        http://people.FreeBSD.org/~roam/roam.key.asc
Key fingerprint FDBA FD79 C26F 3C51 C95E  DF9E ED18 B68D 1619 4553
If I were you, who would be reading this sentence?

Index: src/usr.sbin/diskpart/diskpart.c
===================================================================
RCS file: /home/ncvs/src/usr.sbin/diskpart/Attic/diskpart.c,v
retrieving revision 1.11.2.1
diff -u -r1.11.2.1 diskpart.c
--- src/usr.sbin/diskpart/diskpart.c    7 Jan 2002 06:00:23 -0000       1.11.2.1
+++ src/usr.sbin/diskpart/diskpart.c    2 Dec 2002 11:32:58 -0000
@@ -128,8 +128,6 @@
        char *lp, *tyname;
 
        argc--, argv++;
-       if (argc < 1)
-               usage();
        if (argc > 0 && strcmp(*argv, "-p") == 0) {
                pflag++;
                argc--, argv++;
@@ -140,8 +138,10 @@
        }
        if (argc > 1 && strcmp(*argv, "-s") == 0) {
                totsize = atoi(argv[1]);
-               argc += 2, argv += 2;
+               argc -= 2, argv += 2;
        }
+       if (argc < 1)
+               usage();
        dp = getdiskbyname(*argv);
        if (dp == NULL) {
                if (isatty(0))

Index: src/usr.sbin/diskpart/diskpart.c
===================================================================
RCS file: /home/ncvs/src/usr.sbin/diskpart/Attic/diskpart.c,v
retrieving revision 1.11.2.1
diff -u -r1.11.2.1 diskpart.c
--- src/usr.sbin/diskpart/diskpart.c    7 Jan 2002 06:00:23 -0000       1.11.2.1
+++ src/usr.sbin/diskpart/diskpart.c    20 Nov 2002 15:14:46 -0000
@@ -55,6 +55,7 @@
 #include <ctype.h>
 #include <err.h>
 #include <stdio.h>
+#include <unistd.h>
 
 #define        for_now                 /* show all of `c' partition for disklabel */
 #define        NPARTITIONS     8
@@ -126,22 +127,30 @@
        int threshhold, numcyls[NPARTITIONS], startcyl[NPARTITIONS];
        int totsize = 0;
        char *lp, *tyname;
+       int ch;
 
-       argc--, argv++;
+       while ((ch = getopt(argc, argv, "dps:")) != EOF)
+               switch (ch) {
+                       case 'd':
+                               dflag++;
+                               if (pflag)
+                                       usage();
+                               break;
+                               
+                       case 'p':
+                               if (dflag)
+                                       usage();
+                               pflag++;
+                               break;
+
+                       case 's':
+                               totsize = atoi(optarg);
+                               break;
+               }
+       argc -= optind;
+       argv += optind;
        if (argc < 1)
                usage();
-       if (argc > 0 && strcmp(*argv, "-p") == 0) {
-               pflag++;
-               argc--, argv++;
-       }
-       if (argc > 0 && strcmp(*argv, "-d") == 0) {
-               dflag++;
-               argc--, argv++;
-       }
-       if (argc > 1 && strcmp(*argv, "-s") == 0) {
-               totsize = atoi(argv[1]);
-               argc += 2, argv += 2;
-       }
        dp = getdiskbyname(*argv);
        if (dp == NULL) {
                if (isatty(0))

msg38427/pgp00000.pgp
Description: PGP signature