[mapserver-dev] Buffer overflow in msOWSParseRequestMetadata

[Lime, Steve D (DNR)]

Can LiveDVD wait for a 6.0.4? I think something like this would justify a maintenance release. Alan/Dan would know how quickly that could happen for Ubuntu.

-----Original Message-----
From: mapserver-dev-bounces at lists.osgeo.org [mailto:mapserver-dev-bounces at lists.osgeo.org] On Behalf Of Fabian Schindler
Sent: Tuesday, July 17, 2012 8:01 AM
To: thomas bonfort
Cc: mapserver-dev at lists.osgeo.org
Subject: Re: [mapserver-dev] Buffer overflow in msOWSParseRequestMetadata

Thomas,

Agreed. Let me know if I can help somewhere!

Fabian

On 07/17/2012 02:38 PM, thomas bonfort wrote:
> ok, I'm taking back my offer, I can't for the life of me figure out 
> what the code is doing to try to replicate it differently in a manner 
> that would be compatible with a release branch. For 6.0 I'd just 
> augment the buffer size as you did to correct your issue. For 6.2 the 
> area would need some refactoring to at least address these issues:
>
> - int msOWSRequestIsEnabled(mapObj *map, layerObj *layer, const char 
> *namespaces, const char *request, int check_all_layers) is always 
> called with layer=NULL and check_all_layers=true
> - int msOWSParseRequestMetadata(const char *metadata, const char 
> *request, int *disabled) has the fixed buffer issue you mentionned, 
> and will set *disabled to true even if the request is explicitely 
> enabled
> - void msOWSRequestLayersEnabled(mapObj *map, const char *namespaces, 
> const char *request, owsRequestObj *ows_request) is malloc'ing but not 
> initializing ows_request->enabled_layers which is suspicious.
>
>  From my limited understanding of this area of the code, I would 
> suggest modifying msOWSParseRequestMetadata to int 
> msOWSParseRequestMetadata(const char *metadata, const char *request), 
> returning either MS_TRUE, MS_FALSE or MS_UNKNOWN and let the higher 
> level code make a decision based on that answer.
>
> --
> thomas
>
> On Tue, Jul 17, 2012 at 1:06 PM, Fabian Schindler 
> <fabian.schindler at eox.at>  wrote:
>> Thomas,
>>
>> PS: can you open an issue for this ?
>>
>> Done: https://github.com/mapserver/mapserver/issues/4393
>>
>> Augmenting the buffer size isn't an appropriate fix imho. We can have 
>> two different fixes:
>> - char requestBuffer = msSmallMalloc(strlen(metadata)+1); instead of 
>> char requestBuffer[32]; This has the inconvenience of calling a 
>> malloc.
>>
>> The function is actually called quite often, and using dynamic memory 
>> allocation may result in a worse performance.
>>
>> - The code there seems quite complicated, and if I understand 
>> correctly what it's trying to do can be replaced with some calls to 
>> strcasestr. As Alan is absent this week, I'd be willing to implement 
>> and apply a patch provided you can help validate it doesn't have 
>> side-effects I did not foresee?
>>
>> That would be great! I'm willing to help on the validation and testing.
>>
>>
>> As for 6.0.4 there are no immediate plans I think. Can you package
>> 6.0.3 + a patch for the liveDVD?
>>
>> Actually we use the pre-installed version of MapServer on the 
>> liveDVD, I'm not yet sure how we can solve the issue for us. Perfect 
>> would be a 6.0.4 release on UbuntuGIS, but I'm afraid that won't 
>> happen in time :)
>>
>> Thanks a lot,
>> Fabian
>>
>> _______________________________________________
>> mapserver-dev mailing list
>> mapserver-dev at lists.osgeo.org
>> http://lists.osgeo.org/mailman/listinfo/mapserver-dev
>>

_______________________________________________
mapserver-dev mailing list
mapserver-dev at lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/mapserver-dev