diff options
author | djm@openbsd.org <djm@openbsd.org> | 2018-08-13 02:41:05 +0000 |
---|---|---|
committer | Damien Miller <djm@mindrot.org> | 2018-08-13 12:42:13 +1000 |
commit | c3903c38b0fd168ab3d925c2b129d1a599593426 (patch) | |
tree | a0914654d1d42e32084afe2d34144c0fadc2735d | |
parent | 1b9dd4aa15208100fbc3650f33ea052255578282 (diff) |
upstream: revert compat.[ch] section of the following change. It
causes double-free under some circumstances.
--
date: 2018/07/31 03:07:24; author: djm; state: Exp; lines: +33 -18; commitid: f7g4UI8eeOXReTPh;
fix some memory leaks spotted by Coverity via Jakub Jelen in bz#2366
feedback and ok dtucker@
OpenBSD-Commit-ID: 1e77547f60fdb5e2ffe23e2e4733c54d8d2d1137
-rw-r--r-- | compat.c | 51 | ||||
-rw-r--r-- | compat.h | 14 | ||||
-rw-r--r-- | sshconnect2.c | 15 | ||||
-rw-r--r-- | sshd.c | 10 |
4 files changed, 34 insertions, 56 deletions
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: compat.c,v 1.112 2018/07/31 03:07:24 djm Exp $ */ | 1 | /* $OpenBSD: compat.c,v 1.113 2018/08/13 02:41:05 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl. All rights reserved. | 3 | * Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl. All rights reserved. |
4 | * | 4 | * |
@@ -184,17 +184,13 @@ proto_spec(const char *spec) | |||
184 | } | 184 | } |
185 | 185 | ||
186 | char * | 186 | char * |
187 | compat_cipher_proposal(char *cipher_prop, u_int compat) | 187 | compat_cipher_proposal(char *cipher_prop) |
188 | { | 188 | { |
189 | char *cp; | 189 | if (!(datafellows & SSH_BUG_BIGENDIANAES)) |
190 | |||
191 | if (!(compat & SSH_BUG_BIGENDIANAES)) | ||
192 | return cipher_prop; | 190 | return cipher_prop; |
193 | debug2("%s: original cipher proposal: %s", __func__, cipher_prop); | 191 | debug2("%s: original cipher proposal: %s", __func__, cipher_prop); |
194 | if ((cp = match_filter_blacklist(cipher_prop, "aes*")) == NULL) | 192 | if ((cipher_prop = match_filter_blacklist(cipher_prop, "aes*")) == NULL) |
195 | fatal("match_filter_blacklist failed"); | 193 | fatal("match_filter_blacklist failed"); |
196 | free(cipher_prop); | ||
197 | cipher_prop = cp; | ||
198 | debug2("%s: compat cipher proposal: %s", __func__, cipher_prop); | 194 | debug2("%s: compat cipher proposal: %s", __func__, cipher_prop); |
199 | if (*cipher_prop == '\0') | 195 | if (*cipher_prop == '\0') |
200 | fatal("No supported ciphers found"); | 196 | fatal("No supported ciphers found"); |
@@ -202,17 +198,13 @@ compat_cipher_proposal(char *cipher_prop, u_int compat) | |||
202 | } | 198 | } |
203 | 199 | ||
204 | char * | 200 | char * |
205 | compat_pkalg_proposal(char *pkalg_prop, u_int compat) | 201 | compat_pkalg_proposal(char *pkalg_prop) |
206 | { | 202 | { |
207 | char *cp; | 203 | if (!(datafellows & SSH_BUG_RSASIGMD5)) |
208 | |||
209 | if (!(compat & SSH_BUG_RSASIGMD5)) | ||
210 | return pkalg_prop; | 204 | return pkalg_prop; |
211 | debug2("%s: original public key proposal: %s", __func__, pkalg_prop); | 205 | debug2("%s: original public key proposal: %s", __func__, pkalg_prop); |
212 | if ((cp = match_filter_blacklist(pkalg_prop, "ssh-rsa")) == NULL) | 206 | if ((pkalg_prop = match_filter_blacklist(pkalg_prop, "ssh-rsa")) == NULL) |
213 | fatal("match_filter_blacklist failed"); | 207 | fatal("match_filter_blacklist failed"); |
214 | free(pkalg_prop); | ||
215 | pkalg_prop = cp; | ||
216 | debug2("%s: compat public key proposal: %s", __func__, pkalg_prop); | 208 | debug2("%s: compat public key proposal: %s", __func__, pkalg_prop); |
217 | if (*pkalg_prop == '\0') | 209 | if (*pkalg_prop == '\0') |
218 | fatal("No supported PK algorithms found"); | 210 | fatal("No supported PK algorithms found"); |
@@ -220,31 +212,24 @@ compat_pkalg_proposal(char *pkalg_prop, u_int compat) | |||
220 | } | 212 | } |
221 | 213 | ||
222 | char * | 214 | char * |
223 | compat_kex_proposal(char *kex_prop, u_int compat) | 215 | compat_kex_proposal(char *p) |
224 | { | 216 | { |
225 | char *cp; | 217 | if ((datafellows & (SSH_BUG_CURVE25519PAD|SSH_OLD_DHGEX)) == 0) |
226 | 218 | return p; | |
227 | if ((compat & (SSH_BUG_CURVE25519PAD|SSH_OLD_DHGEX)) == 0) | 219 | debug2("%s: original KEX proposal: %s", __func__, p); |
228 | return kex_prop; | 220 | if ((datafellows & SSH_BUG_CURVE25519PAD) != 0) |
229 | debug2("%s: original KEX proposal: %s", __func__, kex_prop); | 221 | if ((p = match_filter_blacklist(p, |
230 | if ((compat & SSH_BUG_CURVE25519PAD) != 0) { | ||
231 | if ((cp = match_filter_blacklist(kex_prop, | ||
232 | "curve25519-sha256@libssh.org")) == NULL) | 222 | "curve25519-sha256@libssh.org")) == NULL) |
233 | fatal("match_filter_blacklist failed"); | 223 | fatal("match_filter_blacklist failed"); |
234 | free(kex_prop); | 224 | if ((datafellows & SSH_OLD_DHGEX) != 0) { |
235 | kex_prop = cp; | 225 | if ((p = match_filter_blacklist(p, |
236 | } | ||
237 | if ((compat & SSH_OLD_DHGEX) != 0) { | ||
238 | if ((cp = match_filter_blacklist(kex_prop, | ||
239 | "diffie-hellman-group-exchange-sha256," | 226 | "diffie-hellman-group-exchange-sha256," |
240 | "diffie-hellman-group-exchange-sha1")) == NULL) | 227 | "diffie-hellman-group-exchange-sha1")) == NULL) |
241 | fatal("match_filter_blacklist failed"); | 228 | fatal("match_filter_blacklist failed"); |
242 | free(kex_prop); | ||
243 | kex_prop = cp; | ||
244 | } | 229 | } |
245 | debug2("%s: compat KEX proposal: %s", __func__, kex_prop); | 230 | debug2("%s: compat KEX proposal: %s", __func__, p); |
246 | if (*kex_prop == '\0') | 231 | if (*p == '\0') |
247 | fatal("No supported key exchange algorithms found"); | 232 | fatal("No supported key exchange algorithms found"); |
248 | return kex_prop; | 233 | return p; |
249 | } | 234 | } |
250 | 235 | ||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: compat.h,v 1.53 2018/07/31 03:07:24 djm Exp $ */ | 1 | /* $OpenBSD: compat.h,v 1.54 2018/08/13 02:41:05 djm Exp $ */ |
2 | 2 | ||
3 | /* | 3 | /* |
4 | * Copyright (c) 1999, 2000, 2001 Markus Friedl. All rights reserved. | 4 | * Copyright (c) 1999, 2000, 2001 Markus Friedl. All rights reserved. |
@@ -65,15 +65,9 @@ | |||
65 | 65 | ||
66 | u_int compat_datafellows(const char *); | 66 | u_int compat_datafellows(const char *); |
67 | int proto_spec(const char *); | 67 | int proto_spec(const char *); |
68 | 68 | char *compat_cipher_proposal(char *); | |
69 | /* | 69 | char *compat_pkalg_proposal(char *); |
70 | * compat_*_proposal will update their respective proposals based on the | 70 | char *compat_kex_proposal(char *); |
71 | * active compat flags. The replacement is performed in-place - i.e. they | ||
72 | * will free their argument and return a new heap-allocated string. | ||
73 | */ | ||
74 | char *compat_cipher_proposal(char *, u_int compat); | ||
75 | char *compat_pkalg_proposal(char *, u_int compat); | ||
76 | char *compat_kex_proposal(char *, u_int compat); | ||
77 | 71 | ||
78 | extern int datafellows; | 72 | extern int datafellows; |
79 | #endif | 73 | #endif |
diff --git a/sshconnect2.c b/sshconnect2.c index 93192d186..10e4f0a08 100644 --- a/sshconnect2.c +++ b/sshconnect2.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: sshconnect2.c,v 1.283 2018/07/31 03:07:24 djm Exp $ */ | 1 | /* $OpenBSD: sshconnect2.c,v 1.284 2018/08/13 02:41:05 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright (c) 2000 Markus Friedl. All rights reserved. | 3 | * Copyright (c) 2000 Markus Friedl. All rights reserved. |
4 | * Copyright (c) 2008 Damien Miller. All rights reserved. | 4 | * Copyright (c) 2008 Damien Miller. All rights reserved. |
@@ -167,11 +167,11 @@ ssh_kex2(char *host, struct sockaddr *hostaddr, u_short port) | |||
167 | 167 | ||
168 | if ((s = kex_names_cat(options.kex_algorithms, "ext-info-c")) == NULL) | 168 | if ((s = kex_names_cat(options.kex_algorithms, "ext-info-c")) == NULL) |
169 | fatal("%s: kex_names_cat", __func__); | 169 | fatal("%s: kex_names_cat", __func__); |
170 | myproposal[PROPOSAL_KEX_ALGS] = compat_kex_proposal(s, datafellows); | 170 | myproposal[PROPOSAL_KEX_ALGS] = compat_kex_proposal(s); |
171 | myproposal[PROPOSAL_ENC_ALGS_CTOS] = | 171 | myproposal[PROPOSAL_ENC_ALGS_CTOS] = |
172 | compat_cipher_proposal(options.ciphers, datafellows); | 172 | compat_cipher_proposal(options.ciphers); |
173 | myproposal[PROPOSAL_ENC_ALGS_STOC] = | 173 | myproposal[PROPOSAL_ENC_ALGS_STOC] = |
174 | compat_cipher_proposal(options.ciphers, datafellows); | 174 | compat_cipher_proposal(options.ciphers); |
175 | myproposal[PROPOSAL_COMP_ALGS_CTOS] = | 175 | myproposal[PROPOSAL_COMP_ALGS_CTOS] = |
176 | myproposal[PROPOSAL_COMP_ALGS_STOC] = options.compression ? | 176 | myproposal[PROPOSAL_COMP_ALGS_STOC] = options.compression ? |
177 | "zlib@openssh.com,zlib,none" : "none,zlib@openssh.com,zlib"; | 177 | "zlib@openssh.com,zlib,none" : "none,zlib@openssh.com,zlib"; |
@@ -184,15 +184,14 @@ ssh_kex2(char *host, struct sockaddr *hostaddr, u_short port) | |||
184 | fatal("%s: kex_assemble_namelist", __func__); | 184 | fatal("%s: kex_assemble_namelist", __func__); |
185 | free(all_key); | 185 | free(all_key); |
186 | myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = | 186 | myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = |
187 | compat_pkalg_proposal(options.hostkeyalgorithms, | 187 | compat_pkalg_proposal(options.hostkeyalgorithms); |
188 | datafellows); | ||
189 | } else { | 188 | } else { |
190 | /* Enforce default */ | 189 | /* Enforce default */ |
191 | options.hostkeyalgorithms = xstrdup(KEX_DEFAULT_PK_ALG); | 190 | options.hostkeyalgorithms = xstrdup(KEX_DEFAULT_PK_ALG); |
192 | /* Prefer algorithms that we already have keys for */ | 191 | /* Prefer algorithms that we already have keys for */ |
193 | myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = | 192 | myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = |
194 | compat_pkalg_proposal( | 193 | compat_pkalg_proposal( |
195 | order_hostkeyalgs(host, hostaddr, port), datafellows); | 194 | order_hostkeyalgs(host, hostaddr, port)); |
196 | } | 195 | } |
197 | 196 | ||
198 | if (options.rekey_limit || options.rekey_interval) | 197 | if (options.rekey_limit || options.rekey_interval) |
@@ -224,7 +223,7 @@ ssh_kex2(char *host, struct sockaddr *hostaddr, u_short port) | |||
224 | 223 | ||
225 | /* remove ext-info from the KEX proposals for rekeying */ | 224 | /* remove ext-info from the KEX proposals for rekeying */ |
226 | myproposal[PROPOSAL_KEX_ALGS] = | 225 | myproposal[PROPOSAL_KEX_ALGS] = |
227 | compat_kex_proposal(options.kex_algorithms, datafellows); | 226 | compat_kex_proposal(options.kex_algorithms); |
228 | if ((r = kex_prop2buf(kex->my, myproposal)) != 0) | 227 | if ((r = kex_prop2buf(kex->my, myproposal)) != 0) |
229 | fatal("kex_prop2buf: %s", ssh_err(r)); | 228 | fatal("kex_prop2buf: %s", ssh_err(r)); |
230 | 229 | ||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: sshd.c,v 1.513 2018/07/31 03:07:24 djm Exp $ */ | 1 | /* $OpenBSD: sshd.c,v 1.514 2018/08/13 02:41:05 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 |
@@ -2268,11 +2268,11 @@ do_ssh2_kex(void) | |||
2268 | int r; | 2268 | int r; |
2269 | 2269 | ||
2270 | myproposal[PROPOSAL_KEX_ALGS] = compat_kex_proposal( | 2270 | myproposal[PROPOSAL_KEX_ALGS] = compat_kex_proposal( |
2271 | options.kex_algorithms, datafellows); | 2271 | options.kex_algorithms); |
2272 | myproposal[PROPOSAL_ENC_ALGS_CTOS] = compat_cipher_proposal( | 2272 | myproposal[PROPOSAL_ENC_ALGS_CTOS] = compat_cipher_proposal( |
2273 | options.ciphers, datafellows); | 2273 | options.ciphers); |
2274 | myproposal[PROPOSAL_ENC_ALGS_STOC] = compat_cipher_proposal( | 2274 | myproposal[PROPOSAL_ENC_ALGS_STOC] = compat_cipher_proposal( |
2275 | options.ciphers, datafellows); | 2275 | options.ciphers); |
2276 | myproposal[PROPOSAL_MAC_ALGS_CTOS] = | 2276 | myproposal[PROPOSAL_MAC_ALGS_CTOS] = |
2277 | myproposal[PROPOSAL_MAC_ALGS_STOC] = options.macs; | 2277 | myproposal[PROPOSAL_MAC_ALGS_STOC] = options.macs; |
2278 | 2278 | ||
@@ -2286,7 +2286,7 @@ do_ssh2_kex(void) | |||
2286 | options.rekey_interval); | 2286 | options.rekey_interval); |
2287 | 2287 | ||
2288 | myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = compat_pkalg_proposal( | 2288 | myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = compat_pkalg_proposal( |
2289 | list_hostkey_types(), datafellows); | 2289 | list_hostkey_types()); |
2290 | 2290 | ||
2291 | /* start key exchange */ | 2291 | /* start key exchange */ |
2292 | if ((r = kex_setup(active_state, myproposal)) != 0) | 2292 | if ((r = kex_setup(active_state, myproposal)) != 0) |