subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Shahaf <...@daniel.shahaf.name>
Subject Re: [Patch] Fix for Issue 3046: document security requirement for hook script arguments
Date Thu, 19 Jun 2014 23:07:49 GMT
Markus Schaber wrote on Thu, Jun 19, 2014 at 13:12:44 +0000:
> Index: subversion/libsvn_repos/repos.c
> ===================================================================
> --- subversion/libsvn_repos/repos.c	(revision 1603773)
> +++ subversion/libsvn_repos/repos.c	(working copy)
> @@ -280,6 +280,13 @@
>    "# http://svn.apache.org/repos/asf/subversion/trunk/tools/hook-scripts/ and"     
  NL \
>    "# http://svn.apache.org/repos/asf/subversion/trunk/contrib/hook-scripts/"       
  NL
>  
> +#define HOOKS_QUOTE_ARGUMENTS_TEXT                                            \
> +  "# CAUTION:"                                                             NL \
> +  "# For security reasions, you MUST always properly quote arguments when" NL \
> +  "# you use them. For example, a malicious client could try to set a"     NL \
> +  "# revision property named \"foo; rm -rf /;\"."                          NL \

As I said on IRC: the problem values would usually be those that start with a
minus (should be protected against by adding a "--" argument; e.g., see
freeze_freeze() in svnadmin_tests.py) and those that contain whitespace
(should be protected against by "quoting").  Embedded semicolons aren't
usually going to be a problem.

Apart from that, I'd be happy to +1 that based on glasser's thinking
it's a good idea in the issue.  But I think it the text should enumerate
the correct set of problems.

Cheers,

Daniel

P.S. Markus - thanks for forwarding my suggestion on IRC earlier over to
the svnbook issue tracker :)

> +  "# For similar reasons, you should also add a trailing @ to URLs which"  NL \
> +  "# are passed to SVN commands which accept URLs with peg revisions."     NL

Mime
View raw message