Skip to content

Commit

Permalink
upstream commit
Browse files Browse the repository at this point in the history
fix signed/unsigned errors reported by clang-3.7; add
 sshbuf_dup_string() to replace a common idiom of strdup(sshbuf_ptr()) with
 better safety checking; feedback and ok markus@

Upstream-ID: 71f926d9bb3f1efed51319a6daf37e93d57c8820
  • Loading branch information
djmdjm committed May 2, 2016
1 parent d2d6bf8 commit 1a31d02
Show file tree
Hide file tree
Showing 13 changed files with 112 additions and 64 deletions.
6 changes: 3 additions & 3 deletions auth2-chall.c
@@ -1,4 +1,4 @@
/* $OpenBSD: auth2-chall.c,v 1.43 2015/07/18 07:57:14 djm Exp $ */
/* $OpenBSD: auth2-chall.c,v 1.44 2016/05/02 08:49:03 djm Exp $ */
/*
* Copyright (c) 2001 Markus Friedl. All rights reserved.
* Copyright (c) 2001 Per Allansson. All rights reserved.
Expand Down Expand Up @@ -122,8 +122,8 @@ kbdint_alloc(const char *devs)
buffer_append(&b, devices[i]->name,
strlen(devices[i]->name));
}
buffer_append(&b, "\0", 1);
kbdintctxt->devices = xstrdup(buffer_ptr(&b));
if ((kbdintctxt->devices = sshbuf_dup_string(&b)) == NULL)
fatal("%s: sshbuf_dup_string failed", __func__);
buffer_free(&b);
} else {
kbdintctxt->devices = xstrdup(devs);
Expand Down
6 changes: 3 additions & 3 deletions auth2.c
@@ -1,4 +1,4 @@
/* $OpenBSD: auth2.c,v 1.135 2015/01/19 20:07:45 markus Exp $ */
/* $OpenBSD: auth2.c,v 1.136 2016/05/02 08:49:03 djm Exp $ */
/*
* Copyright (c) 2000 Markus Friedl. All rights reserved.
*
Expand Down Expand Up @@ -424,8 +424,8 @@ authmethods_get(Authctxt *authctxt)
buffer_append(&b, authmethods[i]->name,
strlen(authmethods[i]->name));
}
buffer_append(&b, "\0", 1);
list = xstrdup(buffer_ptr(&b));
if ((list = sshbuf_dup_string(&b)) == NULL)
fatal("%s: sshbuf_dup_string failed", __func__);
buffer_free(&b);
return list;
}
Expand Down
7 changes: 4 additions & 3 deletions kex.h
@@ -1,4 +1,4 @@
/* $OpenBSD: kex.h,v 1.76 2016/02/08 10:57:07 djm Exp $ */
/* $OpenBSD: kex.h,v 1.77 2016/05/02 08:49:03 djm Exp $ */

/*
* Copyright (c) 2000, 2001 Markus Friedl. All rights reserved.
Expand Down Expand Up @@ -205,8 +205,9 @@ int kex_ecdh_hash(int, const EC_GROUP *, const char *, const char *,
const u_char *, size_t, const u_char *, size_t, const u_char *, size_t,
const EC_POINT *, const EC_POINT *, const BIGNUM *, u_char *, size_t *);

int kex_c25519_hash(int, const char *, const char *, const char *, size_t,
const char *, size_t, const u_char *, size_t, const u_char *, const u_char *,
int kex_c25519_hash(int, const char *, const char *,
const u_char *, size_t, const u_char *, size_t,
const u_char *, size_t, const u_char *, const u_char *,
const u_char *, size_t, u_char *, size_t *);

void kexc25519_keygen(u_char key[CURVE25519_SIZE], u_char pub[CURVE25519_SIZE])
Expand Down
6 changes: 3 additions & 3 deletions kexc25519.c
@@ -1,4 +1,4 @@
/* $OpenBSD: kexc25519.c,v 1.9 2015/03/26 07:00:04 djm Exp $ */
/* $OpenBSD: kexc25519.c,v 1.10 2016/05/02 08:49:03 djm Exp $ */
/*
* Copyright (c) 2001, 2013 Markus Friedl. All rights reserved.
* Copyright (c) 2010 Damien Miller. All rights reserved.
Expand Down Expand Up @@ -86,8 +86,8 @@ kex_c25519_hash(
int hash_alg,
const char *client_version_string,
const char *server_version_string,
const char *ckexinit, size_t ckexinitlen,
const char *skexinit, size_t skexinitlen,
const u_char *ckexinit, size_t ckexinitlen,
const u_char *skexinit, size_t skexinitlen,
const u_char *serverhostkeyblob, size_t sbloblen,
const u_char client_dh_pub[CURVE25519_SIZE],
const u_char server_dh_pub[CURVE25519_SIZE],
Expand Down
27 changes: 16 additions & 11 deletions monitor.c
@@ -1,4 +1,4 @@
/* $OpenBSD: monitor.c,v 1.158 2016/03/07 19:02:43 djm Exp $ */
/* $OpenBSD: monitor.c,v 1.159 2016/05/02 08:49:03 djm Exp $ */
/*
* Copyright 2002 Niels Provos <provos@citi.umich.edu>
* Copyright 2002 Markus Friedl <markus@openbsd.org>
Expand Down Expand Up @@ -34,6 +34,7 @@

#include <errno.h>
#include <fcntl.h>
#include <limits.h>
#ifdef HAVE_PATHS_H
#include <paths.h>
#endif
Expand Down Expand Up @@ -688,7 +689,8 @@ mm_answer_sign(int sock, Buffer *m)
u_char *p = NULL, *signature = NULL;
char *alg = NULL;
size_t datlen, siglen, alglen;
int r, keyid, is_proof = 0;
int r, is_proof = 0;
u_int keyid;
const char proof_req[] = "hostkeys-prove-00@openssh.com";

debug3("%s", __func__);
Expand All @@ -697,6 +699,8 @@ mm_answer_sign(int sock, Buffer *m)
(r = sshbuf_get_string(m, &p, &datlen)) != 0 ||
(r = sshbuf_get_cstring(m, &alg, &alglen)) != 0)
fatal("%s: buffer error: %s", __func__, ssh_err(r));
if (keyid > INT_MAX)
fatal("%s: invalid key ID", __func__);

/*
* Supported KEX types use SHA1 (20 bytes), SHA256 (32 bytes),
Expand Down Expand Up @@ -1289,7 +1293,8 @@ static int
monitor_valid_userblob(u_char *data, u_int datalen)
{
Buffer b;
char *p, *userstyle;
u_char *p;
char *userstyle, *cp;
u_int len;
int fail = 0;

Expand All @@ -1314,26 +1319,26 @@ monitor_valid_userblob(u_char *data, u_int datalen)
}
if (buffer_get_char(&b) != SSH2_MSG_USERAUTH_REQUEST)
fail++;
p = buffer_get_cstring(&b, NULL);
cp = buffer_get_cstring(&b, NULL);
xasprintf(&userstyle, "%s%s%s", authctxt->user,
authctxt->style ? ":" : "",
authctxt->style ? authctxt->style : "");
if (strcmp(userstyle, p) != 0) {
logit("wrong user name passed to monitor: expected %s != %.100s",
userstyle, p);
if (strcmp(userstyle, cp) != 0) {
logit("wrong user name passed to monitor: "
"expected %s != %.100s", userstyle, cp);
fail++;
}
free(userstyle);
free(p);
free(cp);
buffer_skip_string(&b);
if (datafellows & SSH_BUG_PKAUTH) {
if (!buffer_get_char(&b))
fail++;
} else {
p = buffer_get_cstring(&b, NULL);
if (strcmp("publickey", p) != 0)
cp = buffer_get_cstring(&b, NULL);
if (strcmp("publickey", cp) != 0)
fail++;
free(p);
free(cp);
if (!buffer_get_char(&b))
fail++;
buffer_skip_string(&b);
Expand Down
5 changes: 3 additions & 2 deletions servconf.c
@@ -1,5 +1,5 @@

/* $OpenBSD: servconf.c,v 1.286 2016/03/07 19:02:43 djm Exp $ */
/* $OpenBSD: servconf.c,v 1.287 2016/05/02 08:49:03 djm Exp $ */
/*
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
* All rights reserved
Expand Down Expand Up @@ -2059,7 +2059,8 @@ parse_server_config(ServerOptions *options, const char *filename, Buffer *conf,

debug2("%s: config %s len %d", __func__, filename, buffer_len(conf));

obuf = cbuf = xstrdup(buffer_ptr(conf));
if ((obuf = cbuf = sshbuf_dup_string(conf)) == NULL)
fatal("%s: sshbuf_dup_string failed", __func__);
active = connectinfo ? 0 : 1;
linenum = 1;
while ((cp = strsep(&cbuf, "\n")) != NULL) {
Expand Down
5 changes: 2 additions & 3 deletions sftp-client.c
@@ -1,4 +1,4 @@
/* $OpenBSD: sftp-client.c,v 1.122 2016/04/08 08:19:17 djm Exp $ */
/* $OpenBSD: sftp-client.c,v 1.123 2016/05/02 08:49:03 djm Exp $ */
/*
* Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
*
Expand Down Expand Up @@ -515,8 +515,7 @@ do_lsreaddir(struct sftp_conn *conn, const char *path, int print_flag,
struct sshbuf *msg;
u_int count, id, i, expected_id, ents = 0;
size_t handle_len;
u_char type;
char *handle;
u_char type, *handle;
int status = SSH2_FX_FAILURE;
int r;

Expand Down
15 changes: 8 additions & 7 deletions ssh-agent.c
@@ -1,4 +1,4 @@
/* $OpenBSD: ssh-agent.c,v 1.212 2016/02/15 09:47:49 dtucker Exp $ */
/* $OpenBSD: ssh-agent.c,v 1.213 2016/05/02 08:49:03 djm Exp $ */
/*
* Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
Expand Down Expand Up @@ -144,8 +144,8 @@ char socket_dir[PATH_MAX];
#define LOCK_SALT_SIZE 16
#define LOCK_ROUNDS 1
int locked = 0;
char lock_passwd[LOCK_SIZE];
char lock_salt[LOCK_SALT_SIZE];
u_char lock_pwhash[LOCK_SIZE];
u_char lock_salt[LOCK_SALT_SIZE];

extern char *__progname;

Expand Down Expand Up @@ -677,7 +677,8 @@ static void
process_lock_agent(SocketEntry *e, int lock)
{
int r, success = 0, delay;
char *passwd, passwdhash[LOCK_SIZE];
char *passwd;
u_char passwdhash[LOCK_SIZE];
static u_int fail_count = 0;
size_t pwlen;

Expand All @@ -689,11 +690,11 @@ process_lock_agent(SocketEntry *e, int lock)
if (bcrypt_pbkdf(passwd, pwlen, lock_salt, sizeof(lock_salt),
passwdhash, sizeof(passwdhash), LOCK_ROUNDS) < 0)
fatal("bcrypt_pbkdf");
if (timingsafe_bcmp(passwdhash, lock_passwd, LOCK_SIZE) == 0) {
if (timingsafe_bcmp(passwdhash, lock_pwhash, LOCK_SIZE) == 0) {
debug("agent unlocked");
locked = 0;
fail_count = 0;
explicit_bzero(lock_passwd, sizeof(lock_passwd));
explicit_bzero(lock_pwhash, sizeof(lock_pwhash));
success = 1;
} else {
/* delay in 0.1s increments up to 10s */
Expand All @@ -710,7 +711,7 @@ process_lock_agent(SocketEntry *e, int lock)
locked = 1;
arc4random_buf(lock_salt, sizeof(lock_salt));
if (bcrypt_pbkdf(passwd, pwlen, lock_salt, sizeof(lock_salt),
lock_passwd, sizeof(lock_passwd), LOCK_ROUNDS) < 0)
lock_pwhash, sizeof(lock_pwhash), LOCK_ROUNDS) < 0)
fatal("bcrypt_pbkdf");
success = 1;
}
Expand Down
8 changes: 4 additions & 4 deletions ssh-keygen.c
@@ -1,4 +1,4 @@
/* $OpenBSD: ssh-keygen.c,v 1.288 2016/02/15 09:47:49 dtucker Exp $ */
/* $OpenBSD: ssh-keygen.c,v 1.289 2016/05/02 08:49:03 djm Exp $ */
/*
* Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
Expand Down Expand Up @@ -883,7 +883,7 @@ do_fingerprint(struct passwd *pw)
char *comment = NULL, *cp, *ep, line[SSH_MAX_PUBKEY_BYTES];
int i, invalid = 1;
const char *path;
long int lnum = 0;
u_long lnum = 0;

if (!have_identity)
ask_filename(pw, "Enter file in which the key is");
Expand Down Expand Up @@ -946,7 +946,7 @@ do_fingerprint(struct passwd *pw)
}
/* Retry after parsing leading hostname/key options */
if (public == NULL && (public = try_read_key(&cp)) == NULL) {
debug("%s:%ld: not a public key", path, lnum);
debug("%s:%lu: not a public key", path, lnum);
continue;
}

Expand Down Expand Up @@ -1920,7 +1920,7 @@ do_show_cert(struct passwd *pw)
FILE *f;
char *cp, line[SSH_MAX_PUBKEY_BYTES];
const char *path;
long int lnum = 0;
u_long lnum = 0;

if (!have_identity)
ask_filename(pw, "Enter file in which the key is");
Expand Down
25 changes: 24 additions & 1 deletion sshbuf-misc.c
@@ -1,4 +1,4 @@
/* $OpenBSD: sshbuf-misc.c,v 1.5 2015/10/05 17:11:21 djm Exp $ */
/* $OpenBSD: sshbuf-misc.c,v 1.6 2016/05/02 08:49:03 djm Exp $ */
/*
* Copyright (c) 2011 Damien Miller
*
Expand Down Expand Up @@ -136,3 +136,26 @@ sshbuf_b64tod(struct sshbuf *buf, const char *b64)
return 0;
}

char *
sshbuf_dup_string(struct sshbuf *buf)
{
const u_char *p = NULL, *s = sshbuf_ptr(buf);
size_t l = sshbuf_len(buf);
char *r;

if (s == NULL || l > SIZE_MAX)
return NULL;
/* accept a nul only as the last character in the buffer */
if (l > 0 && (p = memchr(s, '\0', l)) != NULL) {
if (p != s + l - 1)
return NULL;
l--; /* the nul is put back below */
}
if ((r = malloc(l + 1)) == NULL)
return NULL;
if (l > 0)
memcpy(r, s, l);
r[l] = '\0';
return r;
}

9 changes: 8 additions & 1 deletion sshbuf.h
@@ -1,4 +1,4 @@
/* $OpenBSD: sshbuf.h,v 1.6 2015/12/10 07:01:35 mmcc Exp $ */
/* $OpenBSD: sshbuf.h,v 1.7 2016/05/02 08:49:03 djm Exp $ */
/*
* Copyright (c) 2011 Damien Miller
*
Expand Down Expand Up @@ -239,6 +239,13 @@ char *sshbuf_dtob64(struct sshbuf *buf);
/* Decode base64 data and append it to the buffer */
int sshbuf_b64tod(struct sshbuf *buf, const char *b64);

/*
* Duplicate the contents of a buffer to a string (caller to free).
* Returns NULL on buffer error, or if the buffer contains a premature
* nul character.
*/
char *sshbuf_dup_string(struct sshbuf *buf);

/* Macros for decoding/encoding integers */
#define PEEK_U64(p) \
(((u_int64_t)(((const u_char *)(p))[0]) << 56) | \
Expand Down
6 changes: 3 additions & 3 deletions sshconnect2.c
@@ -1,4 +1,4 @@
/* $OpenBSD: sshconnect2.c,v 1.241 2016/04/28 14:30:21 djm Exp $ */
/* $OpenBSD: sshconnect2.c,v 1.242 2016/05/02 08:49:03 djm Exp $ */
/*
* Copyright (c) 2000 Markus Friedl. All rights reserved.
* Copyright (c) 2008 Damien Miller. All rights reserved.
Expand Down Expand Up @@ -1922,8 +1922,8 @@ authmethods_get(void)
buffer_append(&b, method->name, strlen(method->name));
}
}
buffer_append(&b, "\0", 1);
list = xstrdup(buffer_ptr(&b));
if ((list = sshbuf_dup_string(&b)) == NULL)
fatal("%s: sshbuf_dup_string failed", __func__);
buffer_free(&b);
return list;
}
Expand Down

0 comments on commit 1a31d02

Please sign in to comment.