[Buildbot-commits] [Buildbot] #985: wierd behaviour for some custom property values from force build page

[Buildbot]

#985: wierd behaviour for some custom property values from force build page
--------------------+-------------------------------------------------------
Reporter:  rmdugal  |       Owner:       
    Type:  defect   |      Status:  new  
Priority:  major    |   Milestone:  0.8.2
 Version:  0.7.12   |    Keywords:       
--------------------+-------------------------------------------------------
Changes (by dustin):

  * milestone:  undecided => 0.8.2


Comment:

 Here's the offending code:
 {{{
 #!py

 def getAndCheckProperties(req):
     """
 Fetch custom build properties from the HTTP request of a "Force build" or
 "Resubmit build" HTML form.
 Check the names for valid strings, and return None if a problem is found.
 Return a new Properties object containing each property found in req.
 """
     properties = Properties()
     i = 1
     while True:
         pname = req.args.get("property%dname" % i, [""])[0]
         pvalue = req.args.get("property%dvalue" % i, [""])[0]
         if not pname or not pvalue:
             break
         if not re.match(r'^[\w\.\-\/\~:]*$', pname) \
                 or not re.match(r'^[\w\.\-\/\~:]*$', pvalue):
             log.msg("bad property name='%s', value='%s'" % (pname,
 pvalue))
             return None
         properties.setProperty(pname, pvalue, "Force Build Form")
         i = i + 1

     return properties
 }}}
 Obviously this is somewhat security-sensitive, but the use of some regexes
 like this is probably not the best solution.  I'm sure there are harmful
 values that *could* fit through this filter, and you've listed one of many
 useful values that does not fit through it.

 I'm open to suggestions as to how this should be handled.  I'm tempted to
 just scrap the validation entirely, on the theory that no validation
 technique is going to be universally useful to the various projects that
 are built by Buildbot.  Thoughts?

-- 
Ticket URL: <http://buildbot.net/trac/ticket/985#comment:1>
Buildbot <http://buildbot.net/>
Buildbot: build/test automation