[DebianGIS-dev] Bug#523027: incorrect upstream fix for CVE-2009-0840 (mapserver)
Alan Boudreault
aboudreault at mapgears.com
Thu Jun 25 15:12:52 UTC 2009
Hi
I've reported that to the devs. They should fix that as soon as possible.
ALan
On June 22, 2009 09:24:35 am Nico Golde wrote:
> 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.
--
Alan Boudreault
Mapgears
http://www.mapgears.com
More information about the Pkg-grass-devel
mailing list