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
