Re: svn commit: r1429235 - in /subversion/trunk/tools/hook-scripts: validate-files.conf.example validate-files.py

[Daniel Shahaf <d....@daniel.shahaf.name>]

Wow.  I'm probably going to use that on svn.a.o.  However..

breser@apache.org wrote on Sat, Jan 05, 2013 at 08:36:14 -0000:
> +# The command option is the command to run, this command will be run via
> +# the shell of your platform.  Your command will have variable replacement
> +# made on it prior to execution as follows:
> +#  $REPO or ${REPO} expands to the path of the repository for the commit.
> +#  $TXN or ${TXN} expands to the transaction id of the commit.
> +#  $FILE or ${FILE} expands to the name of the file that matched the pattern.
> +#
> +# $ characters that are not followed by one of the above variable names will
> +# be untouched.
> +#
> +# IMPORTANT: AS A CONSEQUENCE OF THE USE OF THE SHELL IT IS IMPORTANT TO
> +# QUOTE THE ARGUMENTS OF YOUR COMMANDS.  THE $FILE VARIABLE DOES CONTAIN
> +# USER GENERATED DATA AND SHELL METACHARACTERS ARE NOT ESCAPED FOR YOU!
> +
> +# The following rule runs the svnauthz command's validate subcommand
> +# for file named authz in the conf subdir if it is present in the commit.
> +# This is a simple way to ensure that invalid authz files are not allowed
> +# to be committed.
> +#[rule:svnauthz-validate]
> +#pattern = conf/authz
> +#command = '%(svnauthz)s' validate -t '$TXN' '$REPO' '$FILE'

This quoting is insufficient, it's still prone to SQL injections.  Since
this is a problem every user of this script would have to solve, how
about having the script ensure that $FILE doesn't contain "'"?

Perhaps make this configurable via a "upon-single-quote = {continue|raise}"
knob in the config file.

Re: svn commit: r1429235 - in /subversion/trunk/tools/hook-scripts: validate-files.conf.example validate-files.py

[Ben Reser <be...@reser.org>]

On Sat, Jan 5, 2013 at 11:17 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> This quoting is insufficient, it's still prone to SQL injections.  Since
> this is a problem every user of this script would have to solve, how
> about having the script ensure that $FILE doesn't contain "'"?
>
> Perhaps make this configurable via a "upon-single-quote = {continue|raise}"
> knob in the config file.

Thanks for the feedback.  Switching to environment variables and
letting the shell expand the variables should resolve that.

Done in r1429444

Re: svn commit: r1429235 - in /subversion/trunk/tools/hook-scripts: validate-files.conf.example validate-files.py

[Ben Reser <be...@reser.org>]

On Sat, Jan 5, 2013 at 11:17 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> This quoting is insufficient, it's still prone to SQL injections.  Since
> this is a problem every user of this script would have to solve, how
> about having the script ensure that $FILE doesn't contain "'"?
>
> Perhaps make this configurable via a "upon-single-quote = {continue|raise}"
> knob in the config file.

Thanks for the feedback.  Switching to environment variables and
letting the shell expand the variables should resolve that.

Done in r1429444