[mapserver-dev] Buffer overflow in msOWSParseRequestMetadata

[Stephan Meißl]

I agree and hope that's feasible since OSGeo-Live is using the unstable
repositories (see [1]) and AFAIK they are aiming at a final release in
August.

I can commit the 6.0 changes if we agree to just increase the buffer
size but would like to wait for Alan for the refactoring in 6.2.

cu
Stephan

[1] http://svn.osgeo.org/osgeo/livedvd/gisvm/trunk/sources.list.d/ubuntugis.list


On Tue, 2012-07-17 at 14:14 +0000, Lime, Steve D (DNR) wrote:
> 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
> 
> 
> _______________________________________________
> mapserver-dev mailing list
> mapserver-dev at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/mapserver-dev