diff options
author | djm@openbsd.org <djm@openbsd.org> | 2016-07-18 06:08:01 +0000 |
---|---|---|
committer | Damien Miller <djm@mindrot.org> | 2016-07-18 16:11:46 +1000 |
commit | eb999a4590846ba4d56ddc90bd07c23abfbab7b1 (patch) | |
tree | b90e5fe6052e03d206dfdfad633dc1cfe5ac6e95 | |
parent | c71ba790c304545464bb494de974cdf0f4b5cf1e (diff) |
upstream commit
Add some unsigned overflow checks for extra_pad. None of
these are reachable with the amount of padding that we use internally.
bz#2566, pointed out by Torben Hansen. ok markus@
Upstream-ID: 4d4be8450ab2fc1b852d5884339f8e8c31c3fd76
-rw-r--r-- | packet.c | 20 |
1 files changed, 15 insertions, 5 deletions
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: packet.c,v 1.232 2016/07/15 05:01:58 dtucker Exp $ */ | 1 | /* $OpenBSD: packet.c,v 1.233 2016/07/18 06:08:01 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> | 3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> |
4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland | 4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland |
@@ -1171,7 +1171,7 @@ ssh_packet_send2_wrapped(struct ssh *ssh) | |||
1171 | { | 1171 | { |
1172 | struct session_state *state = ssh->state; | 1172 | struct session_state *state = ssh->state; |
1173 | u_char type, *cp, macbuf[SSH_DIGEST_MAX_LENGTH]; | 1173 | u_char type, *cp, macbuf[SSH_DIGEST_MAX_LENGTH]; |
1174 | u_char padlen, pad = 0; | 1174 | u_char tmp, padlen, pad = 0; |
1175 | u_int authlen = 0, aadlen = 0; | 1175 | u_int authlen = 0, aadlen = 0; |
1176 | u_int len; | 1176 | u_int len; |
1177 | struct sshenc *enc = NULL; | 1177 | struct sshenc *enc = NULL; |
@@ -1229,14 +1229,24 @@ ssh_packet_send2_wrapped(struct ssh *ssh) | |||
1229 | if (padlen < 4) | 1229 | if (padlen < 4) |
1230 | padlen += block_size; | 1230 | padlen += block_size; |
1231 | if (state->extra_pad) { | 1231 | if (state->extra_pad) { |
1232 | /* will wrap if extra_pad+padlen > 255 */ | 1232 | tmp = state->extra_pad; |
1233 | state->extra_pad = | 1233 | state->extra_pad = |
1234 | roundup(state->extra_pad, block_size); | 1234 | roundup(state->extra_pad, block_size); |
1235 | pad = state->extra_pad - | 1235 | /* check if roundup overflowed */ |
1236 | ((len + padlen) % state->extra_pad); | 1236 | if (state->extra_pad < tmp) |
1237 | return SSH_ERR_INVALID_ARGUMENT; | ||
1238 | tmp = (len + padlen) % state->extra_pad; | ||
1239 | /* Check whether pad calculation below will underflow */ | ||
1240 | if (tmp > state->extra_pad) | ||
1241 | return SSH_ERR_INVALID_ARGUMENT; | ||
1242 | pad = state->extra_pad - tmp; | ||
1237 | DBG(debug3("%s: adding %d (len %d padlen %d extra_pad %d)", | 1243 | DBG(debug3("%s: adding %d (len %d padlen %d extra_pad %d)", |
1238 | __func__, pad, len, padlen, state->extra_pad)); | 1244 | __func__, pad, len, padlen, state->extra_pad)); |
1245 | tmp = padlen; | ||
1239 | padlen += pad; | 1246 | padlen += pad; |
1247 | /* Check whether padlen calculation overflowed */ | ||
1248 | if (padlen < tmp) | ||
1249 | return SSH_ERR_INVALID_ARGUMENT; /* overflow */ | ||
1240 | state->extra_pad = 0; | 1250 | state->extra_pad = 0; |
1241 | } | 1251 | } |
1242 | if ((r = sshbuf_reserve(state->outgoing_packet, padlen, &cp)) != 0) | 1252 | if ((r = sshbuf_reserve(state->outgoing_packet, padlen, &cp)) != 0) |