On Fri, Aug 18, 2017 at 05:33:14PM +0000, Daniel Shahaf wrote: > s...@apache.org wrote on Fri, 18 Aug 2017 10:33 +0000: > > +++ subversion/site/publish/security/CVE-2017-9800-advisory.txt Fri Aug 18 > > 10:33:10 2017 > > @@ -80,24 +96,36 @@ Recommendations: > > the included patch. > > > > 3. Clients that are not able to execute the 'ssh' command are not > > vulnerable. > > ⋮ > > + If the value of this option is set to a non-existent path, then > > svn+ssh:// > > + URLs will no longer work but the svn client will not be vulnerable: > > + ssh = /this/path/does/not/exist > > This workaround does not work if --config-option is passed on the command > line. > > I think it is not unusual to invoke svn(1) through a wrapper that always sets > some options. (In this specific case, someone might have an alias to svn that > sets config:tunnels:ssh differently depending on what jumphost they need to > use.) > > I think the advisory should be complete, i.e., cover every supported use-case, > so it can be used as a checklist that users go through, and once they have > done > so they can be sure that they are no longer vulnerable. That said, I do agree > that a shortened version that covers only the most common use-cases would be a > Good Thing as well. > > Here's suggested text for the former; the latter can be addressed in follow-up > commits: > > Index: CVE-2017-9800-advisory.txt > =================================================================== > --- CVE-2017-9800-advisory.txt (revision 1805448) > +++ CVE-2017-9800-advisory.txt (working copy) > @@ -107,6 +107,9 @@ > If the value of this option is set to a non-existent path, then svn+ssh:// > URLs will no longer work but the svn client will not be vulnerable: > ssh = /this/path/does/not/exist > + This trick WILL NOT WORK if the option setting is overridden by the > + --config-option=config:tunnels:ssh=foo command-line option of the > + command-line client.
Is the same not true for the 'ssh -q --' trick? I would suggest we add a paragraph such as: Index: CVE-2017-9800-advisory.txt =================================================================== --- CVE-2017-9800-advisory.txt (revision 1805402) +++ CVE-2017-9800-advisory.txt (working copy) @@ -123,6 +123,11 @@ Recommendations: either the svn client must be upgraded or svn+ssh:// URLs must be disabled entirely as described above. + Note that the svn client supports the --config-option option which can + be used to override settings specified in the configuration file. + If this option is used to configure the ssh tunnel command, the same + guidelines should be applied to prevent an attack. + The "[tunnels]" section may define additional third-party custom tunnels; those may be vulnerable if they do not perform input validation on their first argument, which contains the hostname to connect to.
