Bug#318833: libpopt-dev: Incorrect handling of leftovers together with poptStuffArgs (buffer overrun and memory leak)

[Richard Levitte - VMS Whacker]

Package: libpopt-dev
Version: 1.7-5
Severity: important
Tags: patch

Hi,

it seems like poptStuffArgs() hasn't been tested very thoroughly.  If
used twice with the same context, it will invariably cause memory
corruption and possibly memory leaks or a crash.

The proper fix is to change the allocation of leftOvers so it adapts
to the input on the fly instead of trying to pre-allocate it in one
go.

Patch attached.

Cheers,
Richard

-- System Information:
Debian Release: testing/unstable
  APT prefers unstable
  APT policy: (500, 'unstable')
Architecture: i386 (i686)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.11-1-686
Locale: LANG=sv_SE, LC_CTYPE=sv_SE (charmap=ISO-8859-1)

Versions of packages libpopt-dev depends on:
ii  libc6-dev [libc-dev]        2.3.2.ds1-22 GNU C Library: Development Librari
ii  libpopt0                    1.7-5        lib for parsing cmdline parameters

libpopt-dev recommends no packages.

-- no debconf information

--- popt.c
+++ popt.c
@@ -168,7 +168,8 @@
     if (!(flags & POPT_CONTEXT_KEEP_FIRST))
        con->os->next = 1;                      /* skip argv[0] */
 
-    con->leftovers = calloc( (argc + 1), sizeof(*con->leftovers) );
+    con->allocLeftovers = argc + 1;
+    con->leftovers = calloc( con->allocLeftovers, sizeof(*con->leftovers) );
     /[EMAIL PROTECTED] [EMAIL PROTECTED]/      /* FIX: W2DO? */
     con->options = options;
     /[EMAIL PROTECTED] [EMAIL PROTECTED]/
@@ -742,7 +743,19 @@
                    return 0;
                }
                if (con->leftovers != NULL)     /* XXX can't happen */
-                   con->leftovers[con->numLeftovers++] = origOptString;
+                   /* One might think we can never overflow the leftovers
+                      array.  Actually, that's true, as long as you don't
+                      use poptStuffArgs()... */
+                   if ((con->numLeftovers + 1) >= (con->allocLeftovers)) {
+                       con->allocLeftovers += 10;
+                       con->leftovers =
+                           realloc(con->leftovers,
+                                   sizeof(*con->leftovers) * 
con->allocLeftovers);
+                   }
+                   con->leftovers[con->numLeftovers++]
+                       = xstrdup(origOptString); /* so a free of a stuffed
+                                                    argv doesn't give us a
+                                                    dangling pointer */
                continue;
            }
 
@@ -1076,7 +1089,11 @@
     }
     con->execs = _free(con->execs);
 
+    for (i = 0; i < con->numLeftovers; i++) {
+       con->leftovers[i] = _free(&con->leftovers[i]);
+    }
     con->leftovers = _free(con->leftovers);
+
     con->finalArgv = _free(con->finalArgv);
     con->appName = _free(con->appName);
     con->otherHelp = _free(con->otherHelp);
--- poptint.h
+++ poptint.h
@@ -65,6 +65,7 @@
 /[EMAIL PROTECTED]@*/ /[EMAIL PROTECTED]@*/
     const char ** leftovers;
     int numLeftovers;
+    int allocLeftovers;
     int nextLeftover;
 /[EMAIL PROTECTED]@*/
     const struct poptOption * options;