[mapserver-dev] Buffer overflow in msOWSParseRequestMetadata

[thomas bonfort]

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
>