Package: release.debian.org Severity: normal Tags: jessie User: release.debian....@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 <guil...@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 <guil...@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 <guil...@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 <guil...@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 <guil...@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 +
signature.asc
Description: PGP signature
