Re: svn commit: r797563 - in /httpd/httpd/trunk: CHANGES support/htdbm.c

[Jeff Trawick <tr...@gmail.com>]

On Fri, Jul 24, 2009 at 1:15 PM, <po...@apache.org> wrote:

> Author: poirier
> Date: Fri Jul 24 17:15:29 2009
> New Revision: 797563
>
> URL: http://svn.apache.org/viewvc?rev=797563&view=rev
> Log:
> htdbm: Fix possible buffer overflow if dbm database has very
> long values.  PR 30586 [Dan Poirier]
>
> PR 30586
> Reported by: Ulf Harnhammar, Swedish IT Incident Centre
>
> Modified:
>    httpd/httpd/trunk/CHANGES
>    httpd/httpd/trunk/support/htdbm.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=797563&r1=797562&r2=797563&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Jul 24 17:15:29 2009
> @@ -11,6 +11,9 @@
>      mod_proxy_ajp: Avoid delivering content from a previous request which
>      failed to send a request body. PR 46949 [Ruediger Pluem]
>
> +  *) htdbm: Fix possible buffer overflow if dbm database has very
> +     long values.  PR 30586 [Dan Poirier]
>

As noted in 2.2.x/STATUS, htdbm_list() now uses unbounded memory:



> @@ -250,8 +250,6 @@
>         fprintf(stderr, "Empty database -- %s\n", htdbm->filename);
>         return APR_ENOENT;
>     }
> -    rec = apr_pcalloc(htdbm->pool, HUGE_STRING_LEN);
> -
>     fprintf(stderr, "Dumping records from database -- %s\n",
> htdbm->filename);
>     fprintf(stderr, "    %-32sComment\n", "Username");
>     while (key.dptr != NULL) {
> @@ -260,11 +258,9 @@
>             fprintf(stderr, "Failed getting data from %s\n",
> htdbm->filename);
>             return APR_EGENERAL;
>         }
> -        strncpy(kb, key.dptr, key.dsize);
> -        kb[key.dsize] = '\0';
> +        kb = apr_pstrndup(htdbm->pool, key.dptr, key.dsize);
>         fprintf(stderr, "    %-32s", kb);
> -        strncpy(rec, val.dptr, val.dsize);
> -        rec[val.dsize] = '\0';
> +        rec = apr_pstrndup(htdbm->pool, val.dptr, val.dsize);
>         cmnt = strchr(rec, ':');
>         if (cmnt)
>             fprintf(stderr, "%s", cmnt + 1);
>

I suggest solving the first allocation by using %.*s%.*s  (the first string
being the key and the second string being a string of blanks the width of
the field).

I suggest solving the second allocation with memchr and %.*s.