[mapserver-dev] Question about the bad mapfile pattern (vulnerability) check

Jeff McKenna jmckenna at gatewaygeomatics.com
Thu Feb 10 14:58:43 PST 2022


Thanks, I am glad we can talk this out.  The more eyes on it all the 
better.  -jeff



On 2022-02-10 4:29 p.m., Steve Lime wrote:
> Patches welcomed! The config files do normalize site-level configuration 
> like this but yes, regexes can be daunting - although they are an 
> important part of configuration in a variety of ways, I don't know that 
> we can simplify things too much. My preferred approach is to use mapfile 
> aliases exclusively and get away from path filtering altogether.
> 
> On Thu, Feb 10, 2022 at 2:18 PM Jeff McKenna 
> <jmckenna at gatewaygeomatics.com <mailto:jmckenna at gatewaygeomatics.com>> 
> wrote:
> 
>     I suppose that is another performance hit.  So cancel that one, ha.
> 
>     Anyway, I do think it's good to discuss the issues, even if nothing
>     else
>     can be done.
> 
>     I'll continue to tackle this on the packaging side.
> 
>     -jeff
> 
> 
> 
>     On 2022-02-10 4:09 p.m., Jeff McKenna wrote:
>      > For example (me thinking out of the box), could the MapServer config
>      > file contain a section where users can specify paths or
>     characters to
>      > disallow (not a regular expression), and then MapServer would
>     convert
>      > that to a regular expression (in either PCRE or libregex syntax) and
>      > apply it at run-time.
>      >
>      > -jeff
>      >
>      >
>      >
>      > On 2022-02-10 4:00 p.m., Jeff McKenna wrote:
>      >> The tricky part here is that most recent packages use the PCRE
>     regular
>      >> expression library, because of its open BSD license, but the "bad"
>      >> pattern hardcoded into the MapServer source is for libregex,
>     which is
>      >> GPL based, and that expression won't work with the PCRE engine.
>      >>
>      >> My thoughts were when the idea of a "MapServer config file" coming,
>      >> was that phew, then users can just point to their valid mapfiles
>     there
>      >> with simple paths or even wildcard paths or directories, and we can
>      >> completely remove that bad pattern stuff from the source code.
>      >>
>      >> So I assumed that the new config file would save us.  In fact the
>      >> config file is just another place to put the bad pattern, which,
>      >> honestly most MapServer users will never understand nor have the
>     time
>      >> to understand.
>      >>
>      >> I am wondering, when we re-look at the bad pattern stuff, if we can
>      >> think of a way that the new config file can replace the hardcoding
>      >> inside the MapServer source.
>      >>
>      >> I hope it's ok to wonder here out loud, ha.
>      >>
>      >> -jeff
>      >>
>      >>
>      >>
>      >>
>      >>
>      >>
>      >>
>      >> On 2022-02-10 3:05 p.m., Tamas Szekeres wrote:
>      >>> Steve,
>      >>>
>      >>> I think we cannot avoid to be platform dependent in this regard. A
>      >>> single slash at the beginning on Windows systems should not be
>      >>> accepted, because it can specify a relative path, however a double
>      >>> back slash at the beginning is accepted (absolute path of a
>     network
>      >>> share). Currently the single slash at the beginning is accepted in
>      >>> all platforms.
>      >>> Applying environment variables might not be a trivial task in
>     all run
>      >>> time environments, so I think the default behavior should work in
>      >>> that way which can do the right thing in most use cases.
>      >>>
>      >>> Best regards,
>      >>>
>      >>> Tamas
>      >>>
>      >>>
>      >>> Steve Lime <sdlime at gmail.com <mailto:sdlime at gmail.com>
>     <mailto:sdlime at gmail.com <mailto:sdlime at gmail.com>>> ezt írta
>      >>> (időpont: 2022. febr. 10., Cs, 15:48):
>      >>>
>      >>>     The idea was to limit things to local paths with no back
>     references
>      >>>     by default. We're not distinguishing between OSes in
>     setting that
>      >>>     pattern. It's possible it's a bit overzealous so we could
>     tweak the
>      >>>     default if that makes sense across operating systems. It can be
>      >>>     overridden by environment variable (or within the config
>     file) and
>      >>>     could be turned off completely with an expression that will
>     never
>      >>> match.
>      >>>
>      >>>     On Thu, Feb 10, 2022 at 4:34 AM Tamas Szekeres
>     <szekerest at gmail.com <mailto:szekerest at gmail.com>
>      >>>     <mailto:szekerest at gmail.com <mailto:szekerest at gmail.com>>>
>     wrote:
>      >>>
>      >>>         Hi Developers,
>      >>>
>      >>>         I noticed that the double back slashes are excluded
>     from the
>      >>>         accepted mapfile pattern in one of the commits not too
>     long ago
>      >>>         according to security vulnerability reasons. The bad patten
>      >>>         regex is now looking like:
>      >>>
>      >>>         const char *ms_map_bad_pattern_default =
>      >>>         "[/\\]{2}|[/\\]?\\.+[/\\]|,";
>      >>>
>      >>>         Do we have a specific reason why we don't accept the
>     double back
>      >>>         slashes at the beginning of the mapfile path? This normally
>      >>>         refers to a network share which is considered to be an
>     absolute
>      >>>         path, and our use cases are working like
>     that extensively. I
>      >>>         guess we wanted to exclude the relative paths
>     basically, but it
>      >>>         seems not to be that case.
>      >>>         I'm also wondering if the double forward slashes at the
>      >>>         beginning makes much sense to exclude, since I think
>     that is
>      >>>         treated as a single forward slash in the unix like
>     systems which
>      >>>         is normally accepted.
>      >>>
>      >>>         Thanks,
>      >>>
>      >>>         Tamas
>      >>>
>      >>>         _______________________________________________
>      >>>         MapServer-dev mailing list
>      >>> MapServer-dev at lists.osgeo.org
>     <mailto:MapServer-dev at lists.osgeo.org>
>      >>> <mailto:MapServer-dev at lists.osgeo.org
>     <mailto:MapServer-dev at lists.osgeo.org>>
>      >>> https://lists.osgeo.org/mailman/listinfo/mapserver-dev
>     <https://lists.osgeo.org/mailman/listinfo/mapserver-dev>
>      >>>         <https://lists.osgeo.org/mailman/listinfo/mapserver-dev
>     <https://lists.osgeo.org/mailman/listinfo/mapserver-dev>>
>      >>>
>      >>>
>      >>> _______________________________________________
>      >>> MapServer-dev mailing list
>      >>> MapServer-dev at lists.osgeo.org
>     <mailto:MapServer-dev at lists.osgeo.org>
>      >>> https://lists.osgeo.org/mailman/listinfo/mapserver-dev
>     <https://lists.osgeo.org/mailman/listinfo/mapserver-dev>
>      >>
>      >>
>      >
>      >
> 
> 
>     -- 
>     Jeff McKenna
>     GatewayGeo: Developers of MS4W, MapServer Consulting and Training
>     co-founder of FOSS4G
>     http://gatewaygeo.com/ <http://gatewaygeo.com/>
>     _______________________________________________
>     MapServer-dev mailing list
>     MapServer-dev at lists.osgeo.org <mailto:MapServer-dev at lists.osgeo.org>
>     https://lists.osgeo.org/mailman/listinfo/mapserver-dev
>     <https://lists.osgeo.org/mailman/listinfo/mapserver-dev>
> 


-- 
Jeff McKenna
GatewayGeo: Developers of MS4W, MapServer Consulting and Training
co-founder of FOSS4G
http://gatewaygeo.com/


More information about the MapServer-dev mailing list