Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 627498 (CVE-2017-12836) - <dev-vcs/cvs-1.12.12-r12: vulnerable to SSH command injection with crafted repo path
Summary: <dev-vcs/cvs-1.12.12-r12: vulnerable to SSH command injection with crafted re...
Status: CONFIRMED
Alias: CVE-2017-12836
Product: Gentoo Security
Classification: Unclassified
Component: Vulnerabilities (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Gentoo Security
URL: http://seclists.org/oss-sec/2017/q3/282
Whiteboard: B2 [glsa cve]
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-10 23:43 UTC by Hank Leininger
Modified: 2019-11-04 11:43 UTC (History)
4 users (show)

See Also:
Package list:
dev-vcs/cvs-1.12.12-r12
Runtime testing required: ---
stable-bot: sanity-check+


Attachments
Patch to add -- to SSH arguments, preventing hostname of -oProxyCommand, etc. (cvs-1.12.12-CVE-2017-12836.patch,1.02 KB, patch)
2017-08-15 19:57 UTC, Hank Leininger
no flags Details | Diff
Ebuild patch to apply the SSH -o fix. (cvs-1.12.12-r12.ebuild.patch,416 bytes, patch)
2017-08-15 19:58 UTC, Hank Leininger
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hank Leininger 2017-08-10 23:43:07 UTC
Just posted about this to oss-security@, relevant here.

SSH command injection via -o... impacts CVS 1.12.x as well, if anybody still cares.

The announcement for git (see bug#627488) mentions CVE-2017-1000117, CVE-2017-9800, and CVE-2017-1000116 for git, Subversion, Mercurial, but makes no mention of CVS.

CVS uses SSH for remote repos when CVS_RSH=ssh.  In which case specifying a hostname of -o... triggers the same sort of thing:

  $ strace -f -e execve cvs -d '-oProxyCommand=id;localhost:/bar' co yada 2>&1 | egrep id
  execve("/usr/bin/cvs", ["cvs", "-d", "-oProxyCommand=id;localhost:/bar", "co", "yada"], 0x7ffe69f75a68 /* 139 vars */) = 0
  [snip]
  [pid 20003] execve("/usr/local/bin/ssh", ["ssh", "-oProxyCommand=id;localhost", "cvs server"], 0x5fb1fc8420 /* 141 vars */
+) = -1 ENOENT (No such file or directory)
  [pid 20003] execve("/usr/bin/ssh", ["ssh", "-oProxyCommand=id;localhost", "cvs server"], 0x5fb1fc8420 /* 141 vars */) = 0
  [pid 20004] execve("/bin/bash", ["/bin/bash", "-c", "exec id;localhost"], 0x32af5f10d0 /* 141 vars */) = 0
  [pid 20004] execve("/usr/bin/id", ["id"], 0xec92226ae0 /* 141 vars */) = 0
  [pid 20004] +++ exited with 0 +++
  [pid 20003] --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=20004, si_uid=3612, si_status=0, si_utime=0, si_stim
+e=0} ---
  ssh_exchange_identification: Connection closed by remote host
  [pid 20003] +++ exited with 255 +++
  --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=20003, si_uid=3612, si_status=255, si_utime=0, si_stime=0} ---

Tested vanilla 1.12.13, and Gentoo 1.12.12-r11.

Of course, the repo specification looks very odd, so tricking a victim may be harder than for SCM tools where it's prefixed by an ssh:// or masked behind a redirect.

I don't see any recent activity on CVS's mailing lists relative to this; may attempt posting there and/or adapting the patches from git & svn if nobody beats me to it.
Comment 1 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2017-08-11 20:30:59 UTC
hlein:
Go ahead and write a quick patch yourself.

You'll need to check:
* The -d option
* $CVSROOT env var
* Repository file when it's read.

As for validations, it's going to be somewhat more annoying.
It can be:
* ":${METHOD}:${SOMETHINGSTR}"
* "${ABSPATH}"
* "${RELATIVE_PATH}"

The relative path is going to pose the most challenge: since it it legal to name a directory with a leading dash.

Maybe just blacklist "-o" at the start of those commands for now, if CVS_RSH is in the environment and non-empty (it might not be 'ssh', as there are use cases for having an ssh wrapper there).
Comment 3 Hank Leininger 2017-08-15 19:57:24 UTC
Created attachment 489222 [details, diff]
Patch to add -- to SSH arguments, preventing hostname of -oProxyCommand, etc.
Comment 4 Hank Leininger 2017-08-15 19:58:37 UTC
Created attachment 489224 [details, diff]
Ebuild patch to apply the SSH -o fix.
Comment 5 Hank Leininger 2017-08-15 20:03:32 UTC
Added a patch that brings in only the rsh-client.c changes from Thorsten Glaser's patches.  His are against CVS 1.12.13, but Gentoo is at 1.12.12+patches, including some Gentoo-local security enhancement patches (command filtering).  His fixes don't apply cleanly to 1.12.12-r11, and Gentoo's patches don't apply cleanly to 1.12.13, so this is the minimum change to our existing version to address this issue.

His patches don't attempt to filter for unwanted things like -o in CVS/Root / $CVSROOT / -d CVSROOT... parameters, instead simply adds a -- after 'ssh' before hostname specification, so that -o... will be treated as a hostname instead of as options, regardless of how it is ingested.  This seems to be a safe approach.

[The patchset that I did not bring in, to root.c, adds more sanity checking, but again, doesn't apply cleanly to our version so I skipped for now.]
Comment 6 Hank Leininger 2017-08-22 18:09:44 UTC
Is there anything I can do to help move this forward?
Comment 7 Hanno Böck gentoo-dev 2017-08-26 15:54:38 UTC
Seems cvs currently has no maintainer in Gentoo.

I have committed a bump with a fix. I have taken the patch from MirBSD, but leaving out the comment changes (they didn't apply cleanly to the upstream code, probably due to other changes made by MirBSD, and they are comment only, so doesn't really matter). We can wait a few days if any issues show up and then stabilize.

For the record, minimalized test case:
cvs -d '-oProxyCommand=gnome-calculator:/' co a

(if gnome-calculator starts you are vulnerable)
Comment 8 Aaron Bauman Gentoo Infrastructure gentoo-dev Security 2017-08-26 16:02:45 UTC
(In reply to Hanno Boeck from comment #7)
> Seems cvs currently has no maintainer in Gentoo.
> 
> I have committed a bump with a fix. I have taken the patch from MirBSD, but
> leaving out the comment changes (they didn't apply cleanly to the upstream
> code, probably due to other changes made by MirBSD, and they are comment
> only, so doesn't really matter). We can wait a few days if any issues show
> up and then stabilize.
> 
> For the record, minimalized test case:
> cvs -d '-oProxyCommand=gnome-calculator:/' co a
> 
> (if gnome-calculator starts you are vulnerable)

Hanno, looking for the bump but I am not seeing it...
Comment 9 Hanno Böck gentoo-dev 2017-08-26 16:29:56 UTC
(In reply to Aaron Bauman from comment #8)
> Hanno, looking for the bump but I am not seeing it...

Forgot git push at first and you were too fast :-) Should be there now:
https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=9aae21baa940cba64b9ca3b26a5cdf69e88fdf2b
Comment 10 Sergei Trofimovich gentoo-dev 2017-08-30 21:43:44 UTC
ia64 stable
Comment 11 Aaron Bauman Gentoo Infrastructure gentoo-dev Security 2017-09-02 19:08:34 UTC
amd64/x86 stable
Comment 12 Tobias Klausmann gentoo-dev 2017-09-04 07:34:44 UTC
Stable on alpha.
Comment 13 Markus Meier gentoo-dev 2017-09-07 19:39:52 UTC
arm stable
Comment 14 Sergei Trofimovich gentoo-dev 2017-09-10 19:48:36 UTC
sparc stable (thanks to Dakon)
Comment 15 Sergei Trofimovich gentoo-dev 2017-09-11 21:52:25 UTC
hppa stable (thanks to Dakon)
Comment 16 Christopher Díaz Riveros (RETIRED) gentoo-dev Security 2017-09-17 20:14:54 UTC
New GLSA Request filed.

@PPC please finish stabilizing.

Gentoo Security Padawan
ChrisADR
Comment 17 Sergei Trofimovich gentoo-dev 2017-09-24 11:01:44 UTC
ppc64 stable
Comment 18 GLSAMaker/CVETool Bot gentoo-dev 2017-09-24 15:44:59 UTC
This issue was resolved and addressed in
 GLSA 201709-17 at https://security.gentoo.org/glsa/201709-17
by GLSA coordinator Aaron Bauman (b-man).
Comment 19 Aaron Bauman Gentoo Infrastructure gentoo-dev Security 2017-09-24 15:45:39 UTC
re-opening for ppc stable and cleanup.
Comment 20 Sergei Trofimovich gentoo-dev 2017-09-24 19:58:27 UTC
ppc stable.

Last arch is done here.
Comment 21 Hank Leininger 2019-10-27 01:17:08 UTC
(In reply to Hanno Boeck from comment #7)
> Seems cvs currently has no maintainer in Gentoo.
> 
> I have committed a bump with a fix. I have taken the patch from MirBSD, but
> leaving out the comment changes (they didn't apply cleanly to the upstream
> code, probably due to other changes made by MirBSD, and they are comment
> only, so doesn't really matter). We can wait a few days if any issues show
> up and then stabilize.
> 
> For the record, minimalized test case:
> cvs -d '-oProxyCommand=gnome-calculator:/' co a
> 
> (if gnome-calculator starts you are vulnerable)

I believe this patchset is incomplete.

This hunk was dropped:

--- cvs-1.12.12.orig/src/rsh-client.c   2005-03-15 10:45:10.000000000 -0700
+++ cvs-1.12.12/src/rsh-client.c        2017-08-15 13:38:29.136095238 -0600
@@ -54,8 +54,9 @@
                        ? root->cvs_server : getenv ("CVS_SERVER"));
     int i = 0;
     /* This needs to fit "rsh", "-b", "-l", "USER", "host",
-       "cmd (w/ args)", and NULL.  We leave some room to grow. */
-    char *rsh_argv[10];
+       "--", "host", "cvs", "-R", "server", and NULL.
+       We leave some room to grow. */
+    char *rsh_argv[16];

     if (!cvs_rsh)
        /* People sometimes suggest or assume that this should default


But this was not a "comment only" change - it also changed from *rsh_argv[10] to*rsh_argv[16].

I'm not sure how much it matters.  I've been using my version which did include the [10] -> [16] change so I can't say that [10] won't be a problem - recall that the rest of the patches included adding an -- argument, probably prompting the increase.

I don't know if there's any other CVS users left; maybe so, and [10] must not be a problem or they'd have complained.  Or maybe I'm the only one, and my local change meant I never used the fix in the main tree.
Comment 22 Hanno Böck gentoo-dev 2019-10-29 13:54:00 UTC
Hi Hank, I think this is a valid issue, I'll have a look.
Comment 23 Hanno Böck gentoo-dev 2019-11-04 11:43:37 UTC
Ultimately the best solution would probably be for us to fetch in Debian's patchset. However this requires sorting out which of our patches are already in there (by a quick check most of them, but not all), and which of the remaining ones should be considered obsolete, kept or maybe submitted to debian as well.