[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090622132435.GA20005@ngolde.de>
Date: Mon, 22 Jun 2009 15:24:35 +0200
From: Nico Golde <oss-security+ml@...lde.de>
To: oss-security@...ts.openwall.com
Cc: aboudreault@...gears.com, coley@...re.org, 523027@...s.debian.org,
warmerdam@...ox.com
Subject: incorrect upstream fix for CVE-2009-0840 (mapserver)
Hi,
from the CVE description:
| Heap-based buffer underflow in the readPostBody function in cgiutil.c in
| mapserv in MapServer 4.x before 4.10.4 and 5.x before 5.2.2 allows remote
| attackers to have an unknown impact via a negative value in the Content-Length
| HTTP header.
The affected code is in cgiutil.c:
41 static char *readPostBody( cgiRequestObj *request )
42 {
43 char *data;
44 int data_max, data_len, chunk_size;
45
46 msIO_needBinaryStdin();
47
48 /* -------------------------------------------------------------------- */
49 /* If the length is provided, read in one gulp. */
50 /* -------------------------------------------------------------------- */
51 if( getenv("CONTENT_LENGTH") != NULL ) {
52 data_max = atoi(getenv("CONTENT_LENGTH"));
53 data = (char *) malloc(data_max+1);
54 if( data == NULL ) {
55 msIO_printf("Content-type: text/html%c%c",10,10);
56 msIO_printf("malloc() failed, Content-Length: %d unreasonably large?\n", data_max );
57 exit( 1 );
58 }
59
60 if( (int) msIO_fread(data, 1, data_max, stdin) < data_max ) {
There is obviously a problem in case the content-length is negative.
The following is the upstream patch which was used to "fix" this issue:
static char *readPostBody( cgiRequestObj *request )
{
char *data;
- int data_max, data_len, chunk_size;
+ unsigned int data_max, data_len;
+ int chunk_size;
Unfortunately this doesn't fix the issue and I wonder why people always think
changing signed types to unsigned will fix such errors.
If I pass 0xffffffff as the content-length according to type conversion rules
in C atoi() will convert this to -1 which is again converted to 0xffff when
assigning it to an unsigned int. data_max+1 in line 53 will then overflow and
malloc is called with a parameter of 0. This causes malloc to allocated the smallest
possible chunk but it will _not_ return NULL (well, implementation defined). So it
is still possible to perform a heap-based buffer overflow after the upstream
fix.
I'm not sure if this should get a new CVE id but the versions in the CVE id
description should be adjusted and the upstream patch revised.
Cheers
Nico
P.S. @Alan, this is also the reason I have to reject your packages in our
security queue again.
--
Nico Golde - http://www.ngolde.de - nion@...ber.ccc.de - GPG: 0xA0A0AAAA
For security reasons, all text in this mail is double-rot13 encrypted.
Content of type "application/pgp-signature" skipped
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
