Considering I’ve become the de-facto upstream of cvs(GNU) even if not yet formally the de-iure upstream maintainer, fixing this bug obviously falls to me — not quite the way I had planned passing this evening after coming home from work and a decent and, worse, very filling meal at the local Croatian restaurant. But, so’s life.

The problem here is basically that CVS invokes ssh(1) (well, rsh originally…) but doesn’t add the argument separator “--” before the (user-provided) hostname, which when starting with a hyphen-minus will be interpreted by ssh as an argument. (Apparently the other VCSes also had additional vulnerabilities such as not properly escaping semicoloi or pipes from the shell or unescaping percent-escaped fun characters, but that doesn’t affect us.)

The obvious fix and the one I implemented first is to simply add the dashes. This will also be backported to Debian {,{,old}old}stable-security.

Then I looked at other VCSes out of which only one did this, but they all added extra paranoia hostname checks (some of them passing invalid hostnames, such as those with underscores in them). OK, I thought, then also let’s add extra checks to CVS’ repository reference handling. This will end up in Debian sid and MirBSD, pending passing the regression tests of course… hah, while writing this article I had to fixup because a test failed. Anyway, it’s not strictly necessary AFAICT to fix the issue.

Update, about 2⅕ hours past midnight (the testsuite runs for several hours): of course, the “sanity” testsuite (which itself is rather insane…) also needs adjustments, plus a bonus fix (for something that got broken when the recent allow-root-regex patch was merged and got fixed in the same go to…night).

tl;dr: a fix will end up in Debian *stable-security and can be taken out of my mail to the bugreport; another few changes for robustness are being tested and then added to both MirBSD and Debian sid. The impact is likely small, as it’s hard to get a user (if you find one, in the first place) to use a crafted CVSROOT string, which is easy to spot as well.

Update, Monday: apparently someone took care of the DSA and DLA yesterday after ACCEPTing the uploads — thanks, I was outside during the day.

Update 2017-08-25: It was noted that ssh(1) does not parse its command line correctly, and therefore the patch above might not be enough in the general case. However, I still think it’s good enough for CVS because it constructs its command line in a way that doesn’t let users exploit that bug.