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

[Even Rouault]

On lundi 7 août 2017 12:50:34 CEST Daniel Morissette wrote:
> Would it not be sufficient to HTML-encode the values before sending them
> out? In this case, the "-->" would become "-->" which would fix the
> vulnerability. We use msEncodeHTMLEntities() for this in mapserv:
> 
> https://github.com/mapserver/mapserver/blob/branch-7-0/mapstring.c#L1225

As double hyphen is prohibited inside XML comments for compatiblity reason, I believe we 
should also (in that context, that alone should be sufficient) escape - as &45; (although 
entities aren't really "official" inside XML comments, but at least this is valid XML)

In other protocols, for example wms, I see the exception is wrapped in a <![CDATA[   bla ]] 
marker. I guess the reason was to avoid escaping the & entities from the user request. I think 
the wrapping inside [CDATA[ should be replaced by standard XML escaping ala 
msEncodeHTMLEntities()

Actually it seems only WMS and WMTS are affected. Other protocols return an error message 
as plaintext.

> 
> Daniel
> 
> On 2017-08-07 12:47 PM, Lime, Steve D (MNIT) wrote:
> > 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/3442086c/attachment.html>