Package: release.debian.org
Severity: normal
Tags: jessie
User: release.debian.org@packages.debian.org
Usertags: pu
Moritz Mühlenhoff from the Security Team suggested to fix dropbear's
known vulnerabilities (CVE-2016-3116 and CVE-2016-740[6-8]) via a point
release, since they don't warrant a DSA.
I enclosed a debdiff against dropbear_2014.65-1.dsc. As mentioned in
the Changelog, the backported patches address the following:
* If X11 forwarding is enabled a user could bypass any "command="
restrictions in authorized_keys and run any command as their own
user.
Cherry-picked from 2016.72, changeset 1229:a3e8389e01ff
https://secure.ucc.asn.au/hg/dropbear/rev/a3e8389e01ff
Fixes CVE-2016-3116
* Message printout was vulnerable to format string injection. If
specific usernames including "%" symbols can be created on a system
then an attacker could run arbitrary code as root when connecting to
Dropbear server.
Cherry-picked from 2016.74, changeset 1304:b66a483f3dcb
https://secure.ucc.asn.au/hg/dropbear/rev/b66a483f3dcb
Fixes CVE-2016-7406
* dropbearconvert import of OpenSSH keys could run arbitrary code as
the local dropbearconvert user when parsing malicious key files
Cherry-picked from 2016.74, changeset 1306:34e6127ef02e, ignoring
space/tab changes.
https://secure.ucc.asn.au/hg/dropbear/rev/34e6127ef02e
Fixes CVE-2016-7407
* dbclient could run arbitrary code as the local dbclient user if
particular -m or -c arguments are provided.
Cherry-picked from 2016.74, changeset 1303:eed9376a4ad6
https://secure.ucc.asn.au/hg/dropbear/rev/eed9376a4ad6
Fixes CVE-2016-7408
Could you consider to have it included in the upcoming point release? (BTW I
was not maintaining dropbear yet when Jessie was released. Therefore -1+deb8u1
looks like an NMU with invalid version number. Should I leave it like this,
should I add the proper suffix, or should I add myself as maintainer?)
Thanks!
Cheers,
--
Guilhem.
diff -u dropbear-2014.65/debian/changelog dropbear-2014.65/debian/changelog
--- dropbear-2014.65/debian/changelog
+++ dropbear-2014.65/debian/changelog
@@ -1,3 +1,19 @@
+dropbear (2014.65-1+deb8u1) jessie; urgency=medium
+
+ * Backport security fix from 2016.72: If X11 forwarding is enabled a user
+ could bypass any "command=" restrictions in authorized_keys and run any
+ command as their own user (CVE-2016-3116).
+ * Backport security fixes from 2016.74:
+ - Message printout was vulnerable to format string injection
+ (CVE-2016-7406).
+ - dropbearconvert import of OpenSSH keys could run arbitrary code as the
+ local dropbearconvert user when parsing malicious key files
+ (CVE-2016-7407).
+ - dbclient could run arbitrary code as the local dbclient user if
+ particular -m or -c arguments are provided (CVE-2016-7408).
+
+ -- Guilhem Moulin <guilhem@guilhem.org> Sat, 28 Jan 2017 18:23:47 +0100
+
dropbear (2014.65-1) unstable; urgency=low
[ Matt Johnston ]
only in patch2:
unchanged:
--- dropbear-2014.65.orig/debian/diff/003-Validate-xauth-input.diff
+++ dropbear-2014.65/debian/diff/003-Validate-xauth-input.diff
@@ -0,0 +1,78 @@
+From 3d0143c962d2514b512b25d2518fa9e5993922e1 Mon Sep 17 00:00:00 2001
+From: Guilhem Moulin <guilhem@guilhem.org>
+Date: Sat, 28 Jan 2017 19:45:24 +0100
+Subject: [PATCH] Validate xauth input
+
+Cherry-picked from upstream changeset 1229:a3e8389e01ff
+https://secure.ucc.asn.au/hg/dropbear/rev/a3e8389e01ff
+
+Fixes CVE-2016-3116. Quoting the 2016.72 release notes,
+
+ If X11 forwarding is enabled a user could bypass any "command="
+ restrictions in authorized_keys and run any command as their own
+ user (or perform other operations allowed by the "xauth" binary such
+ as writing files). It does not affect systems where command=
+ restrictions are not used.
+---
+ svr-x11fwd.c | 27 +++++++++++++++++++++++++--
+ 1 file changed, 25 insertions(+), 2 deletions(-)
+
+diff --git a/svr-x11fwd.c b/svr-x11fwd.c
+index ceca26a..5a489d4 100644
+--- a/svr-x11fwd.c
++++ b/svr-x11fwd.c
+@@ -42,11 +42,29 @@ static void x11accept(struct Listener* listener, int sock);
+ static int bindport(int fd);
+ static int send_msg_channel_open_x11(int fd, struct sockaddr_in* addr);
+
++/* Check untrusted xauth strings for metacharacters */
++/* Returns DROPBEAR_SUCCESS/DROPBEAR_FAILURE */
++static int
++xauth_valid_string(const char *s)
++{
++ size_t i;
++
++ for (i = 0; s[i] != '\0'; i++) {
++ if (!isalnum(s[i]) &&
++ s[i] != '.' && s[i] != ':' && s[i] != '/' &&
++ s[i] != '-' && s[i] != '_') {
++ return DROPBEAR_FAILURE;
++ }
++ }
++ return DROPBEAR_SUCCESS;
++}
++
++
+ /* called as a request for a session channel, sets up listening X11 */
+ /* returns DROPBEAR_SUCCESS or DROPBEAR_FAILURE */
+ int x11req(struct ChanSess * chansess) {
+
+- int fd;
++ int fd = -1;
+
+ if (!svr_pubkey_allows_x11fwd()) {
+ return DROPBEAR_FAILURE;
+@@ -62,6 +80,11 @@ int x11req(struct ChanSess * chansess) {
+ chansess->x11authcookie = buf_getstring(ses.payload, NULL);
+ chansess->x11screennum = buf_getint(ses.payload);
+
++ if (xauth_valid_string(chansess->x11authprot) == DROPBEAR_FAILURE ||
++ xauth_valid_string(chansess->x11authcookie) == DROPBEAR_FAILURE) {
++ dropbear_log(LOG_WARNING, "Bad xauth request");
++ goto fail;
++ }
+ /* create listening socket */
+ fd = socket(PF_INET, SOCK_STREAM, 0);
+ if (fd < 0) {
+@@ -159,7 +182,7 @@ void x11setauth(struct ChanSess *chansess) {
+ return;
+ }
+
+- /* popen is a nice function - code is strongly based on OpenSSH's */
++ /* code is strongly based on OpenSSH's */
+ authprog = popen(XAUTH_COMMAND, "w");
+ if (authprog) {
+ fprintf(authprog, "add %s %s %s\n",
+--
+2.11.0
+
only in patch2:
unchanged:
--- dropbear-2014.65.orig/debian/diff/004-Improve-exit-message-formatting.diff
+++ dropbear-2014.65/debian/diff/004-Improve-exit-message-formatting.diff
@@ -0,0 +1,115 @@
+From ce6faa122f49386583dd7d377bd3edb6cd22df76 Mon Sep 17 00:00:00 2001
+From: Guilhem Moulin <guilhem@guilhem.org>
+Date: Sat, 28 Jan 2017 18:17:28 +0100
+Subject: [PATCH] Improve exit message formatting
+
+Cherry-picked from upstream changeset 1304:b66a483f3dcb
+https://secure.ucc.asn.au/hg/dropbear/rev/b66a483f3dcb
+
+Fixes CVE-2016-7406. Quoting the 2016.74 release notes,
+
+ Security: Message printout was vulnerable to format string injection.
+
+ If specific usernames including "%" symbols can be created on a system
+ (validated by getpwnam()) then an attacker could run arbitrary code as root
+ when connecting to Dropbear server.
+
+ A dbclient user who can control username or host arguments could potentially
+ run arbitrary code as the dbclient user. This could be a problem if scripts
+ or webpages pass untrusted input to the dbclient program.
+---
+ cli-main.c | 17 +++++++++++------
+ svr-session.c | 22 ++++++++++++----------
+ 2 files changed, 23 insertions(+), 16 deletions(-)
+
+diff --git a/cli-main.c b/cli-main.c
+index 0b4047c..3f8a43b 100644
+--- a/cli-main.c
++++ b/cli-main.c
+@@ -89,17 +89,22 @@ int main(int argc, char ** argv) {
+ #endif /* DBMULTI stuff */
+
+ static void cli_dropbear_exit(int exitcode, const char* format, va_list param) {
++ char exitmsg[150];
++ char fullmsg[300];
+
+- char fmtbuf[300];
++ /* Note that exit message must be rendered before session cleanup */
+
++ /* Render the formatted exit message */
++ vsnprintf(exitmsg, sizeof(exitmsg), format, param);
++
++ /* Add the prefix depending on session/auth state */
+ if (!sessinitdone) {
+- snprintf(fmtbuf, sizeof(fmtbuf), "Exited: %s",
+- format);
++ snprintf(fullmsg, sizeof(fullmsg), "Exited: %s", exitmsg);
+ } else {
+- snprintf(fmtbuf, sizeof(fmtbuf),
++ snprintf(fullmsg, sizeof(fullmsg),
+ "Connection to %s@%s:%s exited: %s",
+ cli_opts.username, cli_opts.remotehost,
+- cli_opts.remoteport, format);
++ cli_opts.remoteport, exitmsg);
+ }
+
+ /* Do the cleanup first, since then the terminal will be reset */
+@@ -107,7 +112,7 @@ static void cli_dropbear_exit(int exitcode, const char* format, va_list param) {
+ /* Avoid printing onwards from terminal cruft */
+ fprintf(stderr, "\n");
+
+- _dropbear_log(LOG_INFO, fmtbuf, param);
++ dropbear_log(LOG_INFO, "%s", fullmsg);
+ exit(exitcode);
+ }
+
+diff --git a/svr-session.c b/svr-session.c
+index 4d3c058..5d899aa 100644
+--- a/svr-session.c
++++ b/svr-session.c
+@@ -144,30 +144,32 @@ void svr_session(int sock, int childpipe) {
+
+ /* failure exit - format must be <= 100 chars */
+ void svr_dropbear_exit(int exitcode, const char* format, va_list param) {
++ char exitmsg[150];
++ char fullmsg[300];
+
+- char fmtbuf[300];
++ /* Render the formatted exit message */
++ vsnprintf(exitmsg, sizeof(exitmsg), format, param);
+
++ /* Add the prefix depending on session/auth state */
+ if (!sessinitdone) {
+ /* before session init */
+- snprintf(fmtbuf, sizeof(fmtbuf),
+- "Early exit: %s", format);
++ snprintf(fullmsg, sizeof(fullmsg), "Early exit: %s", exitmsg);
+ } else if (ses.authstate.authdone) {
+ /* user has authenticated */
+- snprintf(fmtbuf, sizeof(fmtbuf),
++ snprintf(fullmsg, sizeof(fullmsg),
+ "Exit (%s): %s",
+- ses.authstate.pw_name, format);
++ ses.authstate.pw_name, exitmsg);
+ } else if (ses.authstate.pw_name) {
+ /* we have a potential user */
+- snprintf(fmtbuf, sizeof(fmtbuf),
++ snprintf(fullmsg, sizeof(fullmsg),
+ "Exit before auth (user '%s', %d fails): %s",
+- ses.authstate.pw_name, ses.authstate.failcount, format);
++ ses.authstate.pw_name, ses.authstate.failcount, exitmsg);
+ } else {
+ /* before userauth */
+- snprintf(fmtbuf, sizeof(fmtbuf),
+- "Exit before auth: %s", format);
++ snprintf(fullmsg, sizeof(fullmsg), "Exit before auth: %s", exitmsg);
+ }
+
+- _dropbear_log(LOG_INFO, fmtbuf, param);
++ dropbear_log(LOG_INFO, "%s", fullmsg);
+
+ #ifdef USE_VFORK
+ /* For uclinux only the main server process should cleanup - we don't want
+--
+2.11.0
+
only in patch2:
unchanged:
--- dropbear-2014.65.orig/debian/diff/005-Merge-fixes-from-PuTTY-import.c.diff
+++ dropbear-2014.65/debian/diff/005-Merge-fixes-from-PuTTY-import.c.diff
@@ -0,0 +1,151 @@
+From c81c9206f4c2367d4ed134e0688d39b836bb4167 Mon Sep 17 00:00:00 2001
+From: Guilhem Moulin <guilhem@guilhem.org>
+Date: Sat, 28 Jan 2017 19:24:20 +0100
+Subject: [PATCH] Merge fixes from PuTTY import.c
+
+Cherry-picked from upstream changeset 1306:34e6127ef02e, ignoring space
+vs. tab changes. https://secure.ucc.asn.au/hg/dropbear/rev/34e6127ef02e
+
+Fixes CVE-2016-7407. Quoting the 2016.74 release notes,
+
+ Security: dropbearconvert import of OpenSSH keys could run arbitrary
+ code as the local dropbearconvert user when parsing malicious key
+ files
+---
+ keyimport.c | 56 +++++++++++++++++++++++++++++++++++++++++++-------------
+ 1 file changed, 43 insertions(+), 13 deletions(-)
+
+diff --git a/keyimport.c b/keyimport.c
+index 66a7f55..f487e87 100644
+--- a/keyimport.c
++++ b/keyimport.c
+@@ -60,6 +60,8 @@ static int openssh_write(const char *filename, sign_key *key,
+ static int dropbear_write(const char*filename, sign_key * key);
+ static sign_key *dropbear_read(const char* filename);
+
++static int toint(unsigned u);
++
+ #if 0
+ static int sshcom_encrypted(const char *filename, char **comment);
+ static struct ssh2_userkey *sshcom_read(const char *filename, char *passphrase);
+@@ -241,12 +243,11 @@ static int ber_read_id_len(void *source, int sourcelen,
+ if ((*p & 0x1F) == 0x1F) {
+ *id = 0;
+ while (*p & 0x80) {
+- *id = (*id << 7) | (*p & 0x7F);
+ p++, sourcelen--;
+ if (sourcelen == 0)
+ return -1;
++ *id = (*id << 7) | (*p & 0x7F);
+ }
+- *id = (*id << 7) | (*p & 0x7F);
+ p++, sourcelen--;
+ } else {
+ *id = *p & 0x1F;
+@@ -257,14 +258,16 @@ static int ber_read_id_len(void *source, int sourcelen,
+ return -1;
+
+ if (*p & 0x80) {
++ unsigned len;
+ int n = *p & 0x7F;
+ p++, sourcelen--;
+ if (sourcelen < n)
+ return -1;
+- *length = 0;
++ len = 0;
+ while (n--)
+- *length = (*length << 8) | (*p++);
++ len = (len << 8) | (*p++);
+ sourcelen -= n;
++ *length = toint(len);
+ } else {
+ *length = *p;
+ p++, sourcelen--;
+@@ -584,7 +587,8 @@ static sign_key *openssh_read(const char *filename, char * UNUSED(passphrase))
+ /* Expect the SEQUENCE header. Take its absence as a failure to decrypt. */
+ ret = ber_read_id_len(p, key->keyblob_len, &id, &len, &flags);
+ p += ret;
+- if (ret < 0 || id != 16) {
++ if (ret < 0 || id != 16 || len < 0 ||
++ key->keyblob+key->keyblob_len-p < len) {
+ errmsg = "ASN.1 decoding failure - wrong password?";
+ goto error;
+ }
+@@ -619,7 +623,7 @@ static sign_key *openssh_read(const char *filename, char * UNUSED(passphrase))
+ ret = ber_read_id_len(p, key->keyblob+key->keyblob_len-p,
+ &id, &len, &flags);
+ p += ret;
+- if (ret < 0 || id != 2 ||
++ if (ret < 0 || id != 2 || len < 0 ||
+ key->keyblob+key->keyblob_len-p < len) {
+ errmsg = "ASN.1 decoding failure";
+ goto error;
+@@ -1408,11 +1412,12 @@ int sshcom_encrypted(const char *filename, char **comment)
+ pos = 8;
+ if (key->keyblob_len < pos+4)
+ goto done; /* key is far too short */
+- pos += 4 + GET_32BIT(key->keyblob + pos); /* skip key type */
+- if (key->keyblob_len < pos+4)
++ len = toint(GET_32BIT(key->keyblob + pos));
++ if (len < 0 || len > key->keyblob_len - pos - 4)
+ goto done; /* key is far too short */
+- len = GET_32BIT(key->keyblob + pos); /* find cipher-type length */
+- if (key->keyblob_len < pos+4+len)
++ pos += 4 + len; /* skip key type */
++ len = toint(GET_32BIT(key->keyblob + pos)); /* find cipher-type length */
++ if (len < 0 || len > key->keyblob_len - pos - 4)
+ goto done; /* cipher type string is incomplete */
+ if (len != 4 || 0 != memcmp(key->keyblob + pos + 4, "none", 4))
+ answer = 1;
+@@ -1602,8 +1607,8 @@ sign_key *sshcom_read(const char *filename, char *passphrase)
+ /*
+ * Strip away the containing string to get to the real meat.
+ */
+- len = GET_32BIT(ciphertext);
+- if (len > cipherlen-4) {
++ len = toint(GET_32BIT(ciphertext));
++ if (len < 0 || len > cipherlen-4) {
+ errmsg = "containing string was ill-formed";
+ goto error;
+ }
+@@ -1670,7 +1675,8 @@ sign_key *sshcom_read(const char *filename, char *passphrase)
+ publen = pos;
+ pos += put_mp(blob+pos, x.start, x.bytes);
+ privlen = pos - publen;
+- }
++ } else
++ return NULL;
+
+ dropbear_assert(privlen > 0); /* should have bombed by now if not */
+
+@@ -1907,3 +1913,27 @@ int sshcom_write(const char *filename, sign_key *key,
+ return ret;
+ }
+ #endif /* ssh.com stuff disabled */
++
++/* From PuTTY misc.c */
++static int toint(unsigned u)
++{
++ /*
++ * Convert an unsigned to an int, without running into the
++ * undefined behaviour which happens by the strict C standard if
++ * the value overflows. You'd hope that sensible compilers would
++ * do the sensible thing in response to a cast, but actually I
++ * don't trust modern compilers not to do silly things like
++ * assuming that _obviously_ you wouldn't have caused an overflow
++ * and so they can elide an 'if (i < 0)' test immediately after
++ * the cast.
++ *
++ * Sensible compilers ought of course to optimise this entire
++ * function into 'just return the input value'!
++ */
++ if (u <= (unsigned)INT_MAX)
++ return (int)u;
++ else if (u >= (unsigned)INT_MIN) /* wrap in cast _to_ unsigned is OK */
++ return INT_MIN + (int)(u - (unsigned)INT_MIN);
++ else
++ return INT_MIN; /* fallback; should never occur on binary machines */
++}
+--
+2.11.0
+
only in patch2:
unchanged:
--- dropbear-2014.65.orig/debian/diff/006-Improve-algorithm-list-parsing.diff
+++ dropbear-2014.65/debian/diff/006-Improve-algorithm-list-parsing.diff
@@ -0,0 +1,107 @@
+From a628d54c4f4481c02d7767e2e8767c9a71362ce1 Mon Sep 17 00:00:00 2001
+From: Guilhem Moulin <guilhem@guilhem.org>
+Date: Sat, 28 Jan 2017 19:37:39 +0100
+Subject: [PATCH] Improve algorithm list parsing
+
+Cherry-picked from upstream changeset 1303:eed9376a4ad6
+https://secure.ucc.asn.au/hg/dropbear/rev/eed9376a4ad6
+
+Fixes CVE-2016-7408. Quoting the the 2016.74 release notes,
+
+ Security: dbclient could run arbitrary code as the local dbclient
+ user if particular -m or -c arguments are provided. This could be an
+ issue where dbclient is used in scripts.
+---
+ common-algo.c | 62 +++++++++++++++++++++++++++++------------------------------
+ 1 file changed, 30 insertions(+), 32 deletions(-)
+
+diff --git a/common-algo.c b/common-algo.c
+index e57f37c..e5aaaf1 100644
+--- a/common-algo.c
++++ b/common-algo.c
+@@ -491,21 +491,6 @@ check_algo(const char* algo_name, algo_type *algos)
+ return NULL;
+ }
+
+-static void
+-try_add_algo(const char *algo_name, algo_type *algos,
+- const char *algo_desc, algo_type * new_algos, int *num_ret)
+-{
+- algo_type *match_algo = check_algo(algo_name, algos);
+- if (!match_algo)
+- {
+- dropbear_log(LOG_WARNING, "This Dropbear program does not support '%s' %s algorithm", algo_name, algo_desc);
+- return;
+- }
+-
+- new_algos[*num_ret] = *match_algo;
+- (*num_ret)++;
+-}
+-
+ /* Checks a user provided comma-separated algorithm list for available
+ * options. Any that are not acceptable are removed in-place. Returns the
+ * number of valid algorithms. */
+@@ -513,30 +498,43 @@ int
+ check_user_algos(const char* user_algo_list, algo_type * algos,
+ const char *algo_desc)
+ {
+- algo_type new_algos[MAX_PROPOSED_ALGO];
+- /* this has two passes. first we sweep through the given list of
+- * algorithms and mark them as usable=2 in the algo_type[] array... */
+- int num_ret = 0;
++ algo_type new_algos[MAX_PROPOSED_ALGO+1];
+ char *work_list = m_strdup(user_algo_list);
+- char *last_name = work_list;
++ char *start = work_list;
+ char *c;
+- for (c = work_list; *c; c++)
++ int n;
++ /* So we can iterate and look for null terminator */
++ memset(new_algos, 0x0, sizeof(new_algos));
++ for (c = work_list, n = 0; ; c++)
+ {
+- if (*c == ',')
+- {
++ char oc = *c;
++ if (n >= MAX_PROPOSED_ALGO) {
++ dropbear_exit("Too many algorithms '%s'", user_algo_list);
++ }
++ if (*c == ',' || *c == '\0') {
++ algo_type *match_algo = NULL;
+ *c = '\0';
+- try_add_algo(last_name, algos, algo_desc, new_algos, &num_ret);
++ match_algo = check_algo(start, algos);
++ if (match_algo) {
++ if (check_algo(start, new_algos)) {
++ TRACE(("Skip repeated algorithm '%s'", start))
++ } else {
++ new_algos[n] = *match_algo;
++ n++;
++ }
++ } else {
++ dropbear_log(LOG_WARNING, "This Dropbear program does not support '%s' %s algorithm", start, algo_desc);
++ }
+ c++;
+- last_name = c;
++ start = c;
++ }
++ if (oc == '\0') {
++ break;
+ }
+ }
+- try_add_algo(last_name, algos, algo_desc, new_algos, &num_ret);
+ m_free(work_list);
+-
+- new_algos[num_ret].name = NULL;
+-
+- /* Copy one more as a blank delimiter */
+- memcpy(algos, new_algos, sizeof(*new_algos) * (num_ret+1));
+- return num_ret;
++ /* n+1 to include a null terminator */
++ memcpy(algos, new_algos, sizeof(*new_algos) * (n+1));
++ return n;
+ }
+ #endif /* ENABLE_USER_ALGO_LIST */
+--
+2.11.0
+
Attachment:
signature.asc
Description: PGP signature