[PATCH 06/10] parser: Use full regexps for delegation rules paths

[Finucane, Stephen]

On 26 Dec 13:05, Laurent Pinchart wrote:
> Hi Johannes,
> 
> On Friday 25 December 2015 16:59:50 Johannes Berg wrote:
> > On Thu, 2015-12-24 at 14:06 +0000, Finucane, Stephen wrote:
> > >
> > > > Paths are validated by trying to compile it as a regexp using a
> > > > custom validator.
> > > > 
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > > Signed-off-by: Mauro Carvalho Chehab <mchehab at osg.samsung.com>
> > > 
> > > Some small nits that I can fix myself. Other than that,
> > 
> > A small comment that may or may not be relevant - but there's a bunch
> > of things one can do with regexes, from taking a lot of CPU to taking a
> > lot of memory.
> > 
> > What's the trust model for running regexes? I haven't found it in the
> > patches easily right now. If it's configured only in the config file it
> > should be OK, but if any kind of remote user can configure it then it
> > may need safeguards of some kind?
> > 
> > I'm just thinking of a use case like kernel.org where you don't really
> > even trust the people who are the typical delegates/admins in patchwork
> > for a given project, since they are pretty much just random people the
> > admin doesn't necessarily trust much.
> > 
> > (Or put another way - I'd hate for them to patch out/disable this
> > feature because of security concerns, since I'd want to use it on
> > kernel.org, but I'm not sure the admins would want me configuring
> > arbitrary regexes there)
> 
> I agree with your concerns but haven't given them a thought to be honest. 
> Right now only patchwork admins can changes the rules, but as you mention we 
> might not trust them.
> 
> I've used regexps for convenience, we could possibly replace them with a less 
> dangerous type of pattern. One option I was toying with was to create rules 
> automatically from MAINTAINERS, but I don't think that would be flexible 
> enough.

Could we use fnmatch instead? This is the suggestion on StackOverflow [1] and
documentation for the function suggests that the grammar is a very simple one
without the possibility for backrefs or other "dangerous" things [2].

Stephen

[1] http://stackoverflow.com/a/1998868/613428
[2] https://books.google.com/books?id=GxKWdn7u4w8C&lpg=PA232&dq=fnmatch%20backtracking&pg=PA232#v=onepage&q=fnmatch%20backtracking&f=false