From a485b7f4e0037c3358190bf29857218312b7dbf2ee5591a6d20613b36bbfbb9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Chv=C3=A1tal?= Date: Fri, 11 Jan 2019 17:55:43 +0000 Subject: [PATCH 1/2] Accepting request 664725 from home:pmonrealgonzalez:branches:network - Security fix: [bsc#1121571, CVE-2018-20685] * The scp client allows remote SSH servers to bypass intended access restrictions * Added patch openssh-7.9p1-CVE-2018-20685.patch OBS-URL: https://build.opensuse.org/request/show/664725 OBS-URL: https://build.opensuse.org/package/show/network/openssh?expand=0&rev=169 --- openssh-7.9p1-CVE-2018-20685.patch | 33 ++++++++++++++++++++++++++++++ openssh.changes | 8 ++++++++ openssh.spec | 1 + 3 files changed, 42 insertions(+) create mode 100644 openssh-7.9p1-CVE-2018-20685.patch diff --git a/openssh-7.9p1-CVE-2018-20685.patch b/openssh-7.9p1-CVE-2018-20685.patch new file mode 100644 index 0000000..ac539dc --- /dev/null +++ b/openssh-7.9p1-CVE-2018-20685.patch @@ -0,0 +1,33 @@ +From 6010c0303a422a9c5fa8860c061bf7105eb7f8b2 Mon Sep 17 00:00:00 2001 +From: "djm@openbsd.org" +Date: Fri, 16 Nov 2018 03:03:10 +0000 +Subject: [PATCH] upstream: disallow empty incoming filename or ones that refer + to the + +current directory; based on report/patch from Harry Sintonen + +OpenBSD-Commit-ID: f27651b30eaee2df49540ab68d030865c04f6de9 +--- + scp.c | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/scp.c b/scp.c +index 60682c687..4f3fdcd3d 100644 +--- a/scp.c ++++ b/scp.c +@@ -1,4 +1,4 @@ +-/* $OpenBSD: scp.c,v 1.197 2018/06/01 04:31:48 dtucker Exp $ */ ++/* $OpenBSD: scp.c,v 1.198 2018/11/16 03:03:10 djm Exp $ */ + /* + * scp - secure remote copy. This is basically patched BSD rcp which + * uses ssh to do the data transfer (instead of using rcmd). +@@ -1106,7 +1106,8 @@ sink(int argc, char **argv) + SCREWUP("size out of range"); + size = (off_t)ull; + +- if ((strchr(cp, '/') != NULL) || (strcmp(cp, "..") == 0)) { ++ if (*cp == '\0' || strchr(cp, '/') != NULL || ++ strcmp(cp, ".") == 0 || strcmp(cp, "..") == 0) { + run_err("error: unexpected filename: %s", cp); + exit(1); + } diff --git a/openssh.changes b/openssh.changes index 1a108cc..6e3e41f 100644 --- a/openssh.changes +++ b/openssh.changes @@ -1,3 +1,11 @@ +------------------------------------------------------------------- +Fri Jan 11 15:09:04 UTC 2019 - Pedro Monreal Gonzalez + +- Security fix: [bsc#1121571, CVE-2018-20685] + * The scp client allows remote SSH servers to bypass intended + access restrictions + * Added patch openssh-7.9p1-CVE-2018-20685.patch + ------------------------------------------------------------------- Thu Jan 3 11:44:45 UTC 2019 - Pedro Monreal Gonzalez diff --git a/openssh.spec b/openssh.spec index edb5e59..4e4fd5d 100644 --- a/openssh.spec +++ b/openssh.spec @@ -99,6 +99,7 @@ Patch31: openssh-7.7p1-ldap.patch Patch32: openssh-7.7p1-IPv6_X_forwarding.patch Patch33: openssh-7.7p1-sftp_print_diagnostic_messages.patch Patch34: openssh-openssl-1_0_0-compatibility.patch +Patch35: openssh-7.9p1-CVE-2018-20685.patch BuildRequires: audit-devel BuildRequires: autoconf BuildRequires: groff From be528d6e10e1c9b2d0b0632cc2492e798d991fdc33c7253e7343c58fbe8fa878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Chv=C3=A1tal?= Date: Thu, 17 Jan 2019 08:11:36 +0000 Subject: [PATCH 2/2] Accepting request 666511 from home:pmonrealgonzalez:branches:network - Security fix: * [bsc#1121816, CVE-2019-6109] scp client spoofing via object name * [bsc#1121818, CVE-2019-6110] scp client spoofing via stderr * [bsc#1121821, CVE-2019-6111] scp client missing received object name validation * Added patch openssh-7.9p1-scp-name-validator.patch OBS-URL: https://build.opensuse.org/request/show/666511 OBS-URL: https://build.opensuse.org/package/show/network/openssh?expand=0&rev=170 --- openssh-7.9p1-scp-name-validator.patch | 348 +++++++++++++++++++++++++ openssh.changes | 10 + openssh.spec | 1 + 3 files changed, 359 insertions(+) create mode 100644 openssh-7.9p1-scp-name-validator.patch diff --git a/openssh-7.9p1-scp-name-validator.patch b/openssh-7.9p1-scp-name-validator.patch new file mode 100644 index 0000000..aacdb3d --- /dev/null +++ b/openssh-7.9p1-scp-name-validator.patch @@ -0,0 +1,348 @@ +diff --git a/defines.h b/defines.h +index 8f421306..8b4af9b2 100644 +--- a/defines.h ++++ b/defines.h +@@ -873,4 +873,10 @@ struct winsize { + # define USE_SYSTEM_GLOB + #endif + ++/* ++ * Define to enable additional scp file name validation against ++ * malicious servers. ++ */ ++#define USE_SCP_NAME_VALIDATOR 1 ++ + #endif /* _DEFINES_H */ +diff --git a/progressmeter.c b/progressmeter.c +index fe9bf52e..b2a3a38d 100644 +--- a/progressmeter.c ++++ b/progressmeter.c +@@ -160,7 +160,10 @@ refresh_progress_meter(void) + buf[0] = '\0'; + file_len = win_size - 35; + if (file_len > 0) { +- len = snprintf(buf, file_len + 1, "\r%s", file); ++ char visbuf[MAX_WINSIZE + 1]; ++ (void) snmprintf(visbuf, sizeof(visbuf), ++ NULL, "%s", file); ++ len = snprintf(buf, file_len + 1, "\r%s", visbuf); + if (len < 0) + len = 0; + if (len >= file_len + 1) +diff --git a/regress/scp-ssh-wrapper.sh b/regress/scp-ssh-wrapper.sh +index 59f1ff63..dd48a482 100644 +--- a/regress/scp-ssh-wrapper.sh ++++ b/regress/scp-ssh-wrapper.sh +@@ -51,6 +51,18 @@ badserver_4) + echo "C755 2 file" + echo "X" + ;; ++badserver_5) ++ echo "D0555 0 " ++ echo "X" ++ ;; ++badserver_6) ++ echo "D0555 0 ." ++ echo "X" ++ ;; ++badserver_7) ++ echo "C0755 2 extrafile" ++ echo "X" ++ ;; + *) + set -- $arg + shift +diff --git a/regress/scp.sh b/regress/scp.sh +index 57cc7706..104c89e1 100644 +--- a/regress/scp.sh ++++ b/regress/scp.sh +@@ -25,6 +25,7 @@ export SCP # used in scp-ssh-wrapper.scp + scpclean() { + rm -rf ${COPY} ${COPY2} ${DIR} ${DIR2} + mkdir ${DIR} ${DIR2} ++ chmod 755 ${DIR} ${DIR2} + } + + verbose "$tid: simple copy local file to local file" +@@ -101,7 +102,7 @@ if [ ! -z "$SUDO" ]; then + $SUDO rm ${DIR2}/copy + fi + +-for i in 0 1 2 3 4; do ++for i in 0 1 2 3 4 5 6 7; do + verbose "$tid: disallow bad server #$i" + SCPTESTMODE=badserver_$i + export DIR SCPTESTMODE +@@ -113,6 +114,15 @@ for i in 0 1 2 3 4; do + scpclean + $SCP -r $scpopts somehost:${DATA} ${DIR2} >/dev/null 2>/dev/null + [ -d ${DIR}/dotpathdir ] && fail "allows dir creation outside of subdir" ++ ++ scpclean ++ $SCP -pr $scpopts somehost:${DATA} ${DIR2} >/dev/null 2>/dev/null ++ [ ! -w ${DIR2} ] && fail "allows target root attribute change" ++ ++ scpclean ++ $SCP $scpopts somehost:${DATA} ${DIR2} >/dev/null 2>/dev/null ++ [ -e ${DIR2}/extrafile ] && fail "allows extranous object creation" ++ rm -f ${DIR2}/extrafile + done + + verbose "$tid: detect non-directory target" +diff --git a/scp.c b/scp.c +index eb17c341..da1a3a44 100644 +--- a/scp.c ++++ b/scp.c +@@ -17,6 +17,7 @@ + /* + * Copyright (c) 1999 Theo de Raadt. All rights reserved. + * Copyright (c) 1999 Aaron Campbell. All rights reserved. ++ * Copyright (c) 2019 Harry Sintonen. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions +@@ -87,6 +88,14 @@ + #ifdef HAVE_SYS_TIME_H + # include + #endif ++#ifdef USE_SCP_NAME_VALIDATOR ++# include ++# ifdef USE_SYSTEM_GLOB ++# include ++# else ++# include "openbsd-compat/glob.h" ++# endif ++#endif + #include + #include + +@@ -277,6 +286,18 @@ do_cmd(char *host, char *remuser, int port, char *cmd, int *fdin, int *fdout) + close(pout[0]); + dup2(pin[0], 0); + dup2(pout[1], 1); ++ /* ++ * If we're not expecting output to stderr, redirect it to void. ++ * This helps avoiding output manipulation attacks by malicious ++ * servers. ++ */ ++ if (!verbose_mode) { ++ int fd = open("/dev/null", O_WRONLY); ++ if (fd != -1) { ++ dup2(fd, 2); ++ close(fd); ++ } ++ } + close(pin[0]); + close(pout[1]); + +@@ -380,9 +401,20 @@ int pflag, iamremote, iamrecursive, targetshouldbedirectory; + #define CMDNEEDS 64 + char cmd[CMDNEEDS]; /* must hold "rcp -r -p -d\0" */ + ++#ifdef USE_SCP_NAME_VALIDATOR ++typedef struct { ++ const char *pattern; ++ int depth; ++} SINKDATA; ++#endif ++ + int response(void); + void rsource(char *, struct stat *); ++#ifdef USE_SCP_NAME_VALIDATOR ++void sink(int, char *[], SINKDATA *); ++#else + void sink(int, char *[]); ++#endif + void source(int, char *[]); + void tolocal(int, char *[]); + void toremote(int, char *[]); +@@ -536,7 +568,11 @@ main(int argc, char **argv) + } + if (tflag) { + /* Receive data. */ ++#ifdef USE_SCP_NAME_VALIDATOR ++ sink(argc, argv, NULL); ++#else + sink(argc, argv); ++#endif + exit(errs != 0); + } + if (argc < 2) +@@ -750,6 +786,9 @@ tolocal(int argc, char **argv) + char *bp, *host = NULL, *src = NULL, *suser = NULL; + arglist alist; + int i, r, sport = -1; ++#ifdef USE_SCP_NAME_VALIDATOR ++ SINKDATA sinkdata; ++#endif + + memset(&alist, '\0', sizeof(alist)); + alist.list = NULL; +@@ -793,7 +832,13 @@ tolocal(int argc, char **argv) + continue; + } + free(bp); ++#ifdef USE_SCP_NAME_VALIDATOR ++ sinkdata.pattern = basename(xstrdup(src)); ++ sinkdata.depth = 0; ++ sink(1, argv + argc - 1, &sinkdata); ++#else + sink(1, argv + argc - 1); ++#endif + (void) close(remin); + remin = remout = -1; + } +@@ -968,8 +1013,71 @@ rsource(char *name, struct stat *statp) + (sizeof(type) == 8 && (val) > INT64_MAX) || \ + (sizeof(type) != 4 && sizeof(type) != 8)) + ++#ifdef USE_SCP_NAME_VALIDATOR ++#ifdef GLOB_ALTDIRFUNC ++struct fakedir { ++ struct dirent de; ++#ifdef BROKEN_ONE_BYTE_DIRENT_D_NAME ++ char denamebuf[256]; ++#endif ++ struct dirent tmpde; ++#ifdef BROKEN_ONE_BYTE_DIRENT_D_NAME ++ char tmpdenamebuf[2]; /* only needs to hold "." or ".." */ ++#endif ++ int dirindex; ++}; ++static struct fakedir fakedir; ++static void ++g_closedir(void *ptr) ++{ ++} ++static struct dirent * ++g_readdir(void *ptr) ++{ ++ struct fakedir *fd = ptr; ++ switch (fd->dirindex) { ++ case 1: ++ case 2: ++ strcpy(fd->tmpde.d_name, fd->dirindex == 1 ? "." : ".."); ++ fd->tmpde.d_type = DT_DIR; ++ fd->tmpde.d_ino = fd->dirindex++; ++ return &fd->tmpde; ++ case 3: ++ fd->de.d_ino = fd->dirindex++; ++ return &fd->de; ++ } ++ return NULL; ++} ++static void * ++g_opendir(const char *name) ++{ ++ if (strcmp(name, ".") != 0) { ++ errno = ENOENT; ++ return NULL; ++ } ++ fakedir.dirindex = 1; ++ return &fakedir; ++} ++static int ++g_stat(const char *name, struct stat *st) ++{ ++ if (strcmp(name, fakedir.de.d_name) != 0) { ++ errno = ENOENT; ++ return -1; ++ } ++ memset(st, 0, sizeof(*st)); ++ st->st_mode = fakedir.de.d_type == DT_DIR ? S_IFDIR : S_IFREG; ++ return 0; ++} ++#endif ++#endif ++ + void ++#ifdef USE_SCP_NAME_VALIDATOR ++sink(int argc, char **argv, SINKDATA *sinkdata) ++#else + sink(int argc, char **argv) ++#endif + { + static BUF buffer; + struct stat stb; +@@ -1113,6 +1221,65 @@ sink(int argc, char **argv) + run_err("error: unexpected filename: %s", cp); + exit(1); + } ++ ++#ifdef USE_SCP_NAME_VALIDATOR ++ if (sinkdata) { ++ /* ++ * Validate the item name returned by the server for ++ * attempts to modify the current directory attributes. ++ * ++ * Only allow it on root level and only if it was ++ * explicitly requested by using "host:" or "dirname/." ++ */ ++ if (strcmp(cp, ".") == 0 && ++ (sinkdata->depth != 0 || strcmp(sinkdata->pattern, ".") != 0)) { ++ run_err("error: unexpected filename: %s", cp); ++ exit(1); ++ } ++ } ++#ifdef GLOB_ALTDIRFUNC ++ /* Use glob(3) function to validate the item name against ++ * the last path element (stored in sinkdata->pattern). ++ * ++ * We verify that the items returned at the target ++ * directory level (depth 0) match this pattern. ++ * ++ * While a limited check, it will prevent some of the ++ * potential attacks by a malicious server. ++ */ ++ if (sinkdata && sinkdata->depth == 0) { ++ glob_t gl; ++ int rc; ++#ifdef BROKEN_ONE_BYTE_DIRENT_D_NAME ++ if (strlen(cp) >= 256) { ++#else ++ if (strlen(cp) >= sizeof(fakedir.de.d_name)) { ++#endif ++ run_err("error: excessively long filename: %s", cp); ++ exit(1); ++ } ++ fakedir.de.d_type = buf[0] == 'D' ? DT_DIR : DT_REG; ++ strcpy(fakedir.de.d_name, cp); ++ ++ memset(&gl, 0, sizeof(gl)); ++ gl.gl_closedir = g_closedir; ++ gl.gl_readdir = g_readdir; ++ gl.gl_opendir = g_opendir; ++ gl.gl_lstat = g_stat; ++ gl.gl_stat = g_stat; ++ ++ rc = glob(sinkdata->pattern, GLOB_ALTDIRFUNC|GLOB_NOSORT, NULL, &gl); ++ globfree(&gl); ++ if (rc != 0) { ++ if (rc == GLOB_NOMATCH) ++ run_err("error: unexpected filename: %s", cp); ++ else ++ run_err("error: glob error %d", rc); ++ exit(1); ++ } ++ } ++#endif ++#endif + if (targisdir) { + static char *namebuf; + static size_t cursize; +@@ -1150,7 +1317,15 @@ sink(int argc, char **argv) + goto bad; + } + vect[0] = xstrdup(np); ++#ifdef USE_SCP_NAME_VALIDATOR ++ if (sinkdata) ++ sinkdata->depth++; ++ sink(1, vect, sinkdata); ++ if (sinkdata) ++ sinkdata->depth--; ++#else + sink(1, vect); ++#endif + if (setimes) { + setimes = 0; + if (utimes(vect[0], tv) < 0) diff --git a/openssh.changes b/openssh.changes index 6e3e41f..fe83ec4 100644 --- a/openssh.changes +++ b/openssh.changes @@ -1,3 +1,13 @@ +------------------------------------------------------------------- +Wed Jan 16 14:11:29 UTC 2019 - Pedro Monreal Gonzalez + +- Security fix: + * [bsc#1121816, CVE-2019-6109] scp client spoofing via object name + * [bsc#1121818, CVE-2019-6110] scp client spoofing via stderr + * [bsc#1121821, CVE-2019-6111] scp client missing received object + name validation + * Added patch openssh-7.9p1-scp-name-validator.patch + ------------------------------------------------------------------- Fri Jan 11 15:09:04 UTC 2019 - Pedro Monreal Gonzalez diff --git a/openssh.spec b/openssh.spec index 4e4fd5d..9f18da8 100644 --- a/openssh.spec +++ b/openssh.spec @@ -100,6 +100,7 @@ Patch32: openssh-7.7p1-IPv6_X_forwarding.patch Patch33: openssh-7.7p1-sftp_print_diagnostic_messages.patch Patch34: openssh-openssl-1_0_0-compatibility.patch Patch35: openssh-7.9p1-CVE-2018-20685.patch +Patch36: openssh-7.9p1-scp-name-validator.patch BuildRequires: audit-devel BuildRequires: autoconf BuildRequires: groff