[mapserver-dev] [mapserver-users] XSS vulnerability on the 'layer' parameter of WMTS (mapcache)

[Lime, Steve D (MNIT)]

Moving to just mapserver-dev... I'm guessing Thomas is traveling, bummer.  I suppose we could look at the instances and see if there really is value in returning the user value and then make the decision on approach.

From: Even Rouault [mailto:even.rouault at spatialys.com]
Sent: Monday, August 07, 2017 11:14 AM
To: mapserver-dev at lists.osgeo.org
Cc: Lime, Steve D (MNIT) <steve.lime at state.mn.us>; Jeff McKenna <jmckenna at gatewaygeomatics.com>; mapserver-users at lists.osgeo.org
Subject: Re: [mapserver-dev] [mapserver-users] XSS vulnerability on the 'layer' parameter of WMTS (mapcache)


On lundi 7 août 2017 15:20:01 CEST Lime, Steve D (MNIT) wrote:

> I'd favor the more simple and safer approach. It's not that difficult for

> the user to validate the layers requested against the GetCapabilties

> response. MapServer itself does not return the name of the invalid layer,

> presumably for the exact same reason. Instead you get

> "msWMSLoadGetMapParams(): WMS server error. Invalid layer(s) given in the

> LAYERS parameter. A layer might be disabled for this request. Check

> wms/ows_enable_request settings.".



Agreed. I guess Jeff's remark was perhaps if the client software sends a lot of requests and it is not very convenient to match the responses with the queries which have been fired. For a single request, re-emitting the value in the response exception doesn't bring much, because the user knows what it has requested.

> Even, would you be willing to prepare a patch?



I can. I would have been interested in hearing Thomas' opinion, but given the period of the year he might not been reading us.

I've researched how to safely "escape" arbitrary user data input for inclusion inside <-- --> markers, but couldn't find any solid reference (probably rejeting "--" should be sufficient ?). As there's one function per protocol where error messages are formatted, the sanitizing could potentially be centralized there, which would reduce the amount of changes. The simpler approach I talked about is simpler indeed but requires to identify all the places where a user value is returned in an error message ( mostly grepping for a "%s" in a set_error() call ) : not hard, but with a slight chance of missing something.



Even



--

Spatialys - Geospatial professional services

http://www.spatialys.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/mapserver-dev/attachments/20170807/8929471d/attachment-0001.html>