Skip to content

Commit

Permalink
Merge branch 'jb45025' into 'master'
Browse files Browse the repository at this point in the history
Fix CVE-2018-20685, CVE-2019-6109, and CVE-2019-6111.

See merge request mer-core/openssh!10
  • Loading branch information
Juho Hamalainen committed Mar 19, 2019
2 parents d6ce1ff + 0d9c7a2 commit bac562a
Show file tree
Hide file tree
Showing 6 changed files with 731 additions and 0 deletions.
@@ -0,0 +1,36 @@
From 0e450a89540c4d138f7439f58545948c295e2be7 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 1/5] CVE-2018-20685: 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 60682c68..4f3fdcd3 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);
}
--
2.17.1

@@ -0,0 +1,68 @@
From ac3b99c2e41bf51b25e885830e4587fc0abff5f1 Mon Sep 17 00:00:00 2001
From: Darren Tucker <dtucker@dtucker.net>
Date: Thu, 24 Jan 2019 10:00:20 +1100
Subject: [PATCH 2/5] CVE-2019-6109-0: For broken read/readv comparisons,
poll(RW).

In the cases where we can't compare to read or readv function pointers
for some reason we currently ifdef out the poll() used to block while
waiting for reads or writes, falling back to busy waiting. This restores
the poll() in this case, but has it always check for read or write,
removing an inline ifdef in the process.
---
atomicio.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/atomicio.c b/atomicio.c
index f854a06f..cffa9fa7 100644
--- a/atomicio.c
+++ b/atomicio.c
@@ -57,9 +57,11 @@ atomicio6(ssize_t (*f) (int, void *, size_t), int fd, void *_s, size_t n,
ssize_t res;
struct pollfd pfd;

-#ifndef BROKEN_READ_COMPARISON
pfd.fd = fd;
+#ifndef BROKEN_READ_COMPARISON
pfd.events = f == read ? POLLIN : POLLOUT;
+#else
+ pfd.events = POLLIN|POLLOUT;
#endif
while (n > pos) {
res = (f) (fd, s + pos, n - pos);
@@ -68,9 +70,7 @@ atomicio6(ssize_t (*f) (int, void *, size_t), int fd, void *_s, size_t n,
if (errno == EINTR)
continue;
if (errno == EAGAIN || errno == EWOULDBLOCK) {
-#ifndef BROKEN_READ_COMPARISON
(void)poll(&pfd, 1, -1);
-#endif
continue;
}
return 0;
@@ -114,9 +114,11 @@ atomiciov6(ssize_t (*f) (int, const struct iovec *, int), int fd,
/* Make a copy of the iov array because we may modify it below */
memcpy(iov, _iov, (size_t)iovcnt * sizeof(*_iov));

-#ifndef BROKEN_READV_COMPARISON
pfd.fd = fd;
+#ifndef BROKEN_READV_COMPARISON
pfd.events = f == readv ? POLLIN : POLLOUT;
+#else
+ pfd.events = POLLIN|POLLOUT;
#endif
for (; iovcnt > 0 && iov[0].iov_len > 0;) {
res = (f) (fd, iov, iovcnt);
@@ -125,9 +127,7 @@ atomiciov6(ssize_t (*f) (int, const struct iovec *, int), int fd,
if (errno == EINTR)
continue;
if (errno == EAGAIN || errno == EWOULDBLOCK) {
-#ifndef BROKEN_READV_COMPARISON
(void)poll(&pfd, 1, -1);
-#endif
continue;
}
return 0;
--
2.17.1

286 changes: 286 additions & 0 deletions rpm/0003-CVE-2019-6109-1-upstream-Sanitize-scp-filenames-via-.patch
@@ -0,0 +1,286 @@
From 647d3a2f82f40f4af9e1b8eace5927b2e3362edd Mon Sep 17 00:00:00 2001
From: "dtucker@openbsd.org" <dtucker@openbsd.org>
Date: Wed, 23 Jan 2019 08:01:46 +0000
Subject: [PATCH 3/5] CVE-2019-6109-1: 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
---
atomicio.c | 20 ++++++++++++++-----
progressmeter.c | 53 ++++++++++++++++++++++---------------------------
progressmeter.h | 3 ++-
scp.c | 3 +++
sftp-client.c | 18 ++++++++++-------
5 files changed, 55 insertions(+), 42 deletions(-)

diff --git a/atomicio.c b/atomicio.c
index cffa9fa7..845b328e 100644
--- a/atomicio.c
+++ b/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.
@@ -67,9 +67,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) {
(void)poll(&pfd, 1, -1);
continue;
}
@@ -124,9 +129,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) {
(void)poll(&pfd, 1, -1);
continue;
}
diff --git a/progressmeter.c b/progressmeter.c
index fe9bf52e..add462dd 100644
--- a/progressmeter.c
+++ b/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 bf179dca..8f667806 100644
--- a/progressmeter.h
+++ b/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 4f3fdcd3..e8dae6a9 100644
--- a/scp.c
+++ b/scp.c
@@ -1,4 +1,6 @@
/* $OpenBSD: scp.c,v 1.198 2018/11/16 03:03:10 djm Exp $ */
+/* and */
+/* $OpenBSD: scp.c,v 1.200 2019/01/23 08:01:46 dtucker Exp $ */
/*
* scp - secure remote copy. This is basically patched BSD rcp which
* uses ssh to do the data transfer (instead of using rcmd).
@@ -585,6 +587,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 4986d6d8..e8df4f7b 100644
--- a/sftp-client.c
+++ b/sftp-client.c
@@ -1,4 +1,6 @@
/* $OpenBSD: sftp-client.c,v 1.130 2018/07/31 03:07:24 djm Exp $ */
+/* and */
+/* $OpenBSD: sftp-client.c,v 1.132 2019/01/23 08:01:46 dtucker Exp $ */
/*
* Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
*
@@ -101,7 +103,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 +125,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 +142,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 +161,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");
--
2.17.1

0 comments on commit bac562a

Please sign in to comment.