openssh: add patches for scp and sftp CVEs

- CVE-2018-20685
- CVE-2019-6109
- CVE-2019-6111
This commit is contained in:
Duncaen 2019-03-05 12:05:31 +01:00
parent d05b9905cf
commit 210edbe546
5 changed files with 619 additions and 3 deletions

View file

@ -0,0 +1,33 @@
From 6010c0303a422a9c5fa8860c061bf7105eb7f8b2 Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <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
--- scp.c
+++ 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);
}

View file

@ -0,0 +1,276 @@
From 11b88754cadcad0ba79b4ffcc127223248dccb54 Mon Sep 17 00:00:00 2001
From: "dtucker@openbsd.org" <dtucker@openbsd.org>
Date: Wed, 23 Jan 2019 08:01:46 +0000
Subject: upstream: Sanitize scp filenames via snmprintf. To do this we move
the progressmeter formatting outside of signal handler context and have the
atomicio callback called for EINTR too. bz#2434 with contributions from djm
and jjelen at redhat.com, ok djm@
OpenBSD-Commit-ID: 1af61c1f70e4f3bd8ab140b9f1fa699481db57d8
CVE-2019-6109
Origin: backport, https://anongit.mindrot.org/openssh.git/commit/?id=8976f1c4b2721c26e878151f52bdf346dfe2d54c
Bug-Debian: https://bugs.debian.org/793412
Last-Update: 2019-02-08
Patch-Name: sanitize-scp-filenames-via-snmprintf.patch
---
atomicio.c | 20 ++++++++++++++-----
progressmeter.c | 53 ++++++++++++++++++++++---------------------------
progressmeter.h | 3 ++-
scp.c | 1 +
sftp-client.c | 16 ++++++++-------
5 files changed, 51 insertions(+), 42 deletions(-)
diff --git a/atomicio.c b/atomicio.c
index f854a06f5..d91bd7621 100644
--- atomicio.c
+++ atomicio.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: atomicio.c,v 1.28 2016/07/27 23:18:12 djm Exp $ */
+/* $OpenBSD: atomicio.c,v 1.29 2019/01/23 08:01:46 dtucker Exp $ */
/*
* Copyright (c) 2006 Damien Miller. All rights reserved.
* Copyright (c) 2005 Anil Madhavapeddy. All rights reserved.
@@ -65,9 +65,14 @@ atomicio6(ssize_t (*f) (int, void *, size_t), int fd, void *_s, size_t n,
res = (f) (fd, s + pos, n - pos);
switch (res) {
case -1:
- if (errno == EINTR)
+ if (errno == EINTR) {
+ /* possible SIGALARM, update callback */
+ if (cb != NULL && cb(cb_arg, 0) == -1) {
+ errno = EINTR;
+ return pos;
+ }
continue;
- if (errno == EAGAIN || errno == EWOULDBLOCK) {
+ } else if (errno == EAGAIN || errno == EWOULDBLOCK) {
#ifndef BROKEN_READ_COMPARISON
(void)poll(&pfd, 1, -1);
#endif
@@ -122,9 +127,14 @@ atomiciov6(ssize_t (*f) (int, const struct iovec *, int), int fd,
res = (f) (fd, iov, iovcnt);
switch (res) {
case -1:
- if (errno == EINTR)
+ if (errno == EINTR) {
+ /* possible SIGALARM, update callback */
+ if (cb != NULL && cb(cb_arg, 0) == -1) {
+ errno = EINTR;
+ return pos;
+ }
continue;
- if (errno == EAGAIN || errno == EWOULDBLOCK) {
+ } else if (errno == EAGAIN || errno == EWOULDBLOCK) {
#ifndef BROKEN_READV_COMPARISON
(void)poll(&pfd, 1, -1);
#endif
diff --git a/progressmeter.c b/progressmeter.c
index fe9bf52e4..add462dde 100644
--- progressmeter.c
+++ progressmeter.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: progressmeter.c,v 1.45 2016/06/30 05:17:05 dtucker Exp $ */
+/* $OpenBSD: progressmeter.c,v 1.46 2019/01/23 08:01:46 dtucker Exp $ */
/*
* Copyright (c) 2003 Nils Nordman. All rights reserved.
*
@@ -31,6 +31,7 @@
#include <errno.h>
#include <signal.h>
+#include <stdarg.h>
#include <stdio.h>
#include <string.h>
#include <time.h>
@@ -39,6 +40,7 @@
#include "progressmeter.h"
#include "atomicio.h"
#include "misc.h"
+#include "utf8.h"
#define DEFAULT_WINSIZE 80
#define MAX_WINSIZE 512
@@ -61,7 +63,7 @@ static void setscreensize(void);
void refresh_progress_meter(void);
/* signal handler for updating the progress meter */
-static void update_progress_meter(int);
+static void sig_alarm(int);
static double start; /* start progress */
static double last_update; /* last progress update */
@@ -74,6 +76,7 @@ static long stalled; /* how long we have been stalled */
static int bytes_per_second; /* current speed in bytes per second */
static int win_size; /* terminal window size */
static volatile sig_atomic_t win_resized; /* for window resizing */
+static volatile sig_atomic_t alarm_fired;
/* units for format_size */
static const char unit[] = " KMGT";
@@ -126,9 +129,17 @@ refresh_progress_meter(void)
off_t bytes_left;
int cur_speed;
int hours, minutes, seconds;
- int i, len;
int file_len;
+ if ((!alarm_fired && !win_resized) || !can_output())
+ return;
+ alarm_fired = 0;
+
+ if (win_resized) {
+ setscreensize();
+ win_resized = 0;
+ }
+
transferred = *counter - (cur_pos ? cur_pos : start_pos);
cur_pos = *counter;
now = monotime_double();
@@ -158,16 +169,11 @@ refresh_progress_meter(void)
/* filename */
buf[0] = '\0';
- file_len = win_size - 35;
+ file_len = win_size - 36;
if (file_len > 0) {
- len = snprintf(buf, file_len + 1, "\r%s", file);
- if (len < 0)
- len = 0;
- if (len >= file_len + 1)
- len = file_len;
- for (i = len; i < file_len; i++)
- buf[i] = ' ';
- buf[file_len] = '\0';
+ buf[0] = '\r';
+ snmprintf(buf+1, sizeof(buf)-1 , &file_len, "%*s",
+ file_len * -1, file);
}
/* percent of transfer done */
@@ -228,22 +234,11 @@ refresh_progress_meter(void)
/*ARGSUSED*/
static void
-update_progress_meter(int ignore)
+sig_alarm(int ignore)
{
- int save_errno;
-
- save_errno = errno;
-
- if (win_resized) {
- setscreensize();
- win_resized = 0;
- }
- if (can_output())
- refresh_progress_meter();
-
- signal(SIGALRM, update_progress_meter);
+ signal(SIGALRM, sig_alarm);
+ alarm_fired = 1;
alarm(UPDATE_INTERVAL);
- errno = save_errno;
}
void
@@ -259,10 +254,9 @@ start_progress_meter(const char *f, off_t filesize, off_t *ctr)
bytes_per_second = 0;
setscreensize();
- if (can_output())
- refresh_progress_meter();
+ refresh_progress_meter();
- signal(SIGALRM, update_progress_meter);
+ signal(SIGALRM, sig_alarm);
signal(SIGWINCH, sig_winch);
alarm(UPDATE_INTERVAL);
}
@@ -286,6 +280,7 @@ stop_progress_meter(void)
static void
sig_winch(int sig)
{
+ signal(SIGWINCH, sig_winch);
win_resized = 1;
}
diff --git a/progressmeter.h b/progressmeter.h
index bf179dca6..8f6678060 100644
--- progressmeter.h
+++ progressmeter.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: progressmeter.h,v 1.3 2015/01/14 13:54:13 djm Exp $ */
+/* $OpenBSD: progressmeter.h,v 1.4 2019/01/23 08:01:46 dtucker Exp $ */
/*
* Copyright (c) 2002 Nils Nordman. All rights reserved.
*
@@ -24,4 +24,5 @@
*/
void start_progress_meter(const char *, off_t, off_t *);
+void refresh_progress_meter(void);
void stop_progress_meter(void);
diff --git a/scp.c b/scp.c
index 7163d33dc..80308573c 100644
--- scp.c
+++ scp.c
@@ -593,6 +593,7 @@ scpio(void *_cnt, size_t s)
off_t *cnt = (off_t *)_cnt;
*cnt += s;
+ refresh_progress_meter();
if (limit_kbps > 0)
bandwidth_limit(&bwlimit, s);
return 0;
diff --git a/sftp-client.c b/sftp-client.c
index 4986d6d8d..2bc698f86 100644
--- sftp-client.c
+++ sftp-client.c
@@ -101,7 +101,9 @@ sftpio(void *_bwlimit, size_t amount)
{
struct bwlimit *bwlimit = (struct bwlimit *)_bwlimit;
- bandwidth_limit(bwlimit, amount);
+ refresh_progress_meter();
+ if (bwlimit != NULL)
+ bandwidth_limit(bwlimit, amount);
return 0;
}
@@ -121,8 +123,8 @@ send_msg(struct sftp_conn *conn, struct sshbuf *m)
iov[1].iov_base = (u_char *)sshbuf_ptr(m);
iov[1].iov_len = sshbuf_len(m);
- if (atomiciov6(writev, conn->fd_out, iov, 2,
- conn->limit_kbps > 0 ? sftpio : NULL, &conn->bwlimit_out) !=
+ if (atomiciov6(writev, conn->fd_out, iov, 2, sftpio,
+ conn->limit_kbps > 0 ? &conn->bwlimit_out : NULL) !=
sshbuf_len(m) + sizeof(mlen))
fatal("Couldn't send packet: %s", strerror(errno));
@@ -138,8 +140,8 @@ get_msg_extended(struct sftp_conn *conn, struct sshbuf *m, int initial)
if ((r = sshbuf_reserve(m, 4, &p)) != 0)
fatal("%s: buffer error: %s", __func__, ssh_err(r));
- if (atomicio6(read, conn->fd_in, p, 4,
- conn->limit_kbps > 0 ? sftpio : NULL, &conn->bwlimit_in) != 4) {
+ if (atomicio6(read, conn->fd_in, p, 4, sftpio,
+ conn->limit_kbps > 0 ? &conn->bwlimit_in : NULL) != 4) {
if (errno == EPIPE || errno == ECONNRESET)
fatal("Connection closed");
else
@@ -157,8 +159,8 @@ get_msg_extended(struct sftp_conn *conn, struct sshbuf *m, int initial)
if ((r = sshbuf_reserve(m, msg_len, &p)) != 0)
fatal("%s: buffer error: %s", __func__, ssh_err(r));
- if (atomicio6(read, conn->fd_in, p, msg_len,
- conn->limit_kbps > 0 ? sftpio : NULL, &conn->bwlimit_in)
+ if (atomicio6(read, conn->fd_in, p, msg_len, sftpio,
+ conn->limit_kbps > 0 ? &conn->bwlimit_in : NULL)
!= msg_len) {
if (errno == EPIPE)
fatal("Connection closed");

View file

@ -0,0 +1,187 @@
From 125924e47db3713a85a70e0f8d6c23818d2ea054 Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Sat, 26 Jan 2019 22:41:28 +0000
Subject: upstream: check in scp client that filenames sent during
remote->local directory copies satisfy the wildcard specified by the user.
This checking provides some protection against a malicious server
sending unexpected filenames, but it comes at a risk of rejecting wanted
files due to differences between client and server wildcard expansion rules.
For this reason, this also adds a new -T flag to disable the check.
reported by Harry Sintonen
fix approach suggested by markus@;
has been in snaps for ~1wk courtesy deraadt@
OpenBSD-Commit-ID: 00f44b50d2be8e321973f3c6d014260f8f7a8eda
CVE-2019-6111
Origin: backport, https://anongit.mindrot.org/openssh.git/commit/?id=391ffc4b9d31fa1f4ad566499fef9176ff8a07dc
Last-Update: 2019-02-08
Patch-Name: check-filenames-in-scp-client.patch
---
scp.1 | 12 +++++++++++-
scp.c | 37 +++++++++++++++++++++++++++++--------
2 files changed, 40 insertions(+), 9 deletions(-)
diff --git a/scp.1 b/scp.1
index 0e5cc1b2d..397e77091 100644
--- scp.1
+++ scp.1
@@ -18,7 +18,7 @@
.Nd secure copy (remote file copy program)
.Sh SYNOPSIS
.Nm scp
-.Op Fl 346BCpqrv
+.Op Fl 346BCpqrTv
.Op Fl c Ar cipher
.Op Fl F Ar ssh_config
.Op Fl i Ar identity_file
@@ -208,6 +208,16 @@ to use for the encrypted connection.
The program must understand
.Xr ssh 1
options.
+.It Fl T
+Disable strict filename checking.
+By default when copying files from a remote host to a local directory
+.Nm
+checks that the received filenames match those requested on the command-line
+to prevent the remote end from sending unexpected or unwanted files.
+Because of differences in how various operating systems and shells interpret
+filename wildcards, these checks may cause wanted files to be rejected.
+This option disables these checks at the expense of fully trusting that
+the server will not send unexpected filenames.
.It Fl v
Verbose mode.
Causes
diff --git a/scp.c b/scp.c
index 1971c80cd..035037bcc 100644
--- scp.c
+++ scp.c
@@ -94,6 +94,7 @@
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
+#include <fnmatch.h>
#include <limits.h>
#include <locale.h>
#include <pwd.h>
@@ -383,14 +384,14 @@ void verifydir(char *);
struct passwd *pwd;
uid_t userid;
int errs, remin, remout;
-int pflag, iamremote, iamrecursive, targetshouldbedirectory;
+int Tflag, pflag, iamremote, iamrecursive, targetshouldbedirectory;
#define CMDNEEDS 64
char cmd[CMDNEEDS]; /* must hold "rcp -r -p -d\0" */
int response(void);
void rsource(char *, struct stat *);
-void sink(int, char *[]);
+void sink(int, char *[], const char *);
void source(int, char *[]);
void tolocal(int, char *[]);
void toremote(int, char *[]);
@@ -429,8 +430,9 @@ main(int argc, char **argv)
addargs(&args, "-oRemoteCommand=none");
addargs(&args, "-oRequestTTY=no");
- fflag = tflag = 0;
- while ((ch = getopt(argc, argv, "dfl:prtvBCc:i:P:q12346S:o:F:")) != -1)
+ fflag = Tflag = tflag = 0;
+ while ((ch = getopt(argc, argv,
+ "dfl:prtTvBCc:i:P:q12346S:o:F:")) != -1) {
switch (ch) {
/* User-visible flags. */
case '1':
@@ -509,9 +511,13 @@ main(int argc, char **argv)
setmode(0, O_BINARY);
#endif
break;
+ case 'T':
+ Tflag = 1;
+ break;
default:
usage();
}
+ }
argc -= optind;
argv += optind;
@@ -542,7 +548,7 @@ main(int argc, char **argv)
}
if (tflag) {
/* Receive data. */
- sink(argc, argv);
+ sink(argc, argv, NULL);
exit(errs != 0);
}
if (argc < 2)
@@ -800,7 +806,7 @@ tolocal(int argc, char **argv)
continue;
}
free(bp);
- sink(1, argv + argc - 1);
+ sink(1, argv + argc - 1, src);
(void) close(remin);
remin = remout = -1;
}
@@ -976,7 +982,7 @@ rsource(char *name, struct stat *statp)
(sizeof(type) != 4 && sizeof(type) != 8))
void
-sink(int argc, char **argv)
+sink(int argc, char **argv, const char *src)
{
static BUF buffer;
struct stat stb;
@@ -992,6 +998,7 @@ sink(int argc, char **argv)
unsigned long long ull;
int setimes, targisdir, wrerrno = 0;
char ch, *cp, *np, *targ, *why, *vect[1], buf[2048], visbuf[2048];
+ char *src_copy = NULL, *restrict_pattern = NULL;
struct timeval tv[2];
#define atime tv[0]
@@ -1016,6 +1023,17 @@ sink(int argc, char **argv)
(void) atomicio(vwrite, remout, "", 1);
if (stat(targ, &stb) == 0 && S_ISDIR(stb.st_mode))
targisdir = 1;
+ if (src != NULL && !iamrecursive && !Tflag) {
+ /*
+ * Prepare to try to restrict incoming filenames to match
+ * the requested destination file glob.
+ */
+ if ((src_copy = strdup(src)) == NULL)
+ fatal("strdup failed");
+ if ((restrict_pattern = strrchr(src_copy, '/')) != NULL) {
+ *restrict_pattern++ = '\0';
+ }
+ }
for (first = 1;; first = 0) {
cp = buf;
if (atomicio(read, remin, cp, 1) != 1)
@@ -1120,6 +1138,9 @@ sink(int argc, char **argv)
run_err("error: unexpected filename: %s", cp);
exit(1);
}
+ if (restrict_pattern != NULL &&
+ fnmatch(restrict_pattern, cp, 0) != 0)
+ SCREWUP("filename does not match request");
if (targisdir) {
static char *namebuf;
static size_t cursize;
@@ -1157,7 +1178,7 @@ sink(int argc, char **argv)
goto bad;
}
vect[0] = xstrdup(np);
- sink(1, vect);
+ sink(1, vect, src);
if (setimes) {
setimes = 0;
if (utimes(vect[0], tv) < 0)

View file

@ -0,0 +1,120 @@
From 2a8f710447442e9a03e71c022859112ec2d77d17 Mon Sep 17 00:00:00 2001
From: "dtucker@openbsd.org" <dtucker@openbsd.org>
Date: Thu, 24 Jan 2019 16:52:17 +0000
Subject: upstream: Have progressmeter force an update at the beginning and
end of each transfer. Fixes the problem recently introduces where very quick
transfers do not display the progressmeter at all. Spotted by naddy@
OpenBSD-Commit-ID: 68dc46c259e8fdd4f5db3ec2a130f8e4590a7a9a
Origin: upstream, https://anongit.mindrot.org/openssh.git/commit/?id=bdc6c63c80b55bcbaa66b5fde31c1cb1d09a41eb
Last-Update: 2019-02-08
Patch-Name: have-progressmeter-force-update-at-beginning-and-end-transfer.patch
---
progressmeter.c | 13 +++++--------
progressmeter.h | 4 ++--
scp.c | 2 +-
sftp-client.c | 2 +-
4 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/progressmeter.c b/progressmeter.c
index add462dde..e385c1254 100644
--- progressmeter.c
+++ progressmeter.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: progressmeter.c,v 1.46 2019/01/23 08:01:46 dtucker Exp $ */
+/* $OpenBSD: progressmeter.c,v 1.47 2019/01/24 16:52:17 dtucker Exp $ */
/*
* Copyright (c) 2003 Nils Nordman. All rights reserved.
*
@@ -59,9 +59,6 @@ static void format_rate(char *, int, off_t);
static void sig_winch(int);
static void setscreensize(void);
-/* updates the progressmeter to reflect the current state of the transfer */
-void refresh_progress_meter(void);
-
/* signal handler for updating the progress meter */
static void sig_alarm(int);
@@ -120,7 +117,7 @@ format_size(char *buf, int size, off_t bytes)
}
void
-refresh_progress_meter(void)
+refresh_progress_meter(int force_update)
{
char buf[MAX_WINSIZE + 1];
off_t transferred;
@@ -131,7 +128,7 @@ refresh_progress_meter(void)
int hours, minutes, seconds;
int file_len;
- if ((!alarm_fired && !win_resized) || !can_output())
+ if ((!force_update && !alarm_fired && !win_resized) || !can_output())
return;
alarm_fired = 0;
@@ -254,7 +251,7 @@ start_progress_meter(const char *f, off_t filesize, off_t *ctr)
bytes_per_second = 0;
setscreensize();
- refresh_progress_meter();
+ refresh_progress_meter(1);
signal(SIGALRM, sig_alarm);
signal(SIGWINCH, sig_winch);
@@ -271,7 +268,7 @@ stop_progress_meter(void)
/* Ensure we complete the progress */
if (cur_pos != end_pos)
- refresh_progress_meter();
+ refresh_progress_meter(1);
atomicio(vwrite, STDOUT_FILENO, "\n", 1);
}
diff --git a/progressmeter.h b/progressmeter.h
index 8f6678060..1703ea75b 100644
--- progressmeter.h
+++ progressmeter.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: progressmeter.h,v 1.4 2019/01/23 08:01:46 dtucker Exp $ */
+/* $OpenBSD: progressmeter.h,v 1.5 2019/01/24 16:52:17 dtucker Exp $ */
/*
* Copyright (c) 2002 Nils Nordman. All rights reserved.
*
@@ -24,5 +24,5 @@
*/
void start_progress_meter(const char *, off_t, off_t *);
-void refresh_progress_meter(void);
+void refresh_progress_meter(int);
void stop_progress_meter(void);
diff --git a/scp.c b/scp.c
index 80308573c..1971c80cd 100644
--- scp.c
+++ scp.c
@@ -593,7 +593,7 @@ scpio(void *_cnt, size_t s)
off_t *cnt = (off_t *)_cnt;
*cnt += s;
- refresh_progress_meter();
+ refresh_progress_meter(0);
if (limit_kbps > 0)
bandwidth_limit(&bwlimit, s);
return 0;
diff --git a/sftp-client.c b/sftp-client.c
index 2bc698f86..cf2887a40 100644
--- sftp-client.c
+++ sftp-client.c
@@ -101,7 +101,7 @@ sftpio(void *_bwlimit, size_t amount)
{
struct bwlimit *bwlimit = (struct bwlimit *)_bwlimit;
- refresh_progress_meter();
+ refresh_progress_meter(0);
if (bwlimit != NULL)
bandwidth_limit(bwlimit, amount);
return 0;

View file

@ -1,7 +1,7 @@
# Template file for 'openssh'
pkgname=openssh
version=7.9p1
revision=2
revision=3
build_style=gnu-configure
configure_args="--datadir=/usr/share/openssh
--sysconfdir=/etc/ssh --without-selinux --with-privsep-user=nobody
@ -17,7 +17,7 @@ hostmakedepends="autoconf"
makedepends="libedit-devel pam-devel zlib-devel
$(vopt_if gssapi 'mit-krb5-devel') $(vopt_if ldns 'libldns-devel')
$(vopt_if ssl 'libressl-devel')"
short_desc="The OpenSSH implementation of SSH protocol"
short_desc="OpenSSH free Secure Shell (SSH) client and server implementation"
maintainer="Juan RP <xtraeme@voidlinux.org>"
license="BSD-2-Clause, ISC"
homepage="https://www.openssh.com"
@ -30,7 +30,7 @@ make_dirs="/var/chroot/ssh 0755 root root"
build_options="ldns ssl gssapi"
build_options_default="ldns ssl"
CFLAGS+=" -Wno-format-truncation -Wno-stringop-truncation"
CFLAGS="-Wno-format-truncation -Wno-stringop-truncation"
case $XBPS_TARGET_MACHINE in
i686-musl)