[PATCH] Make SSH2 private key parsing errors fatal
Michael Forney
mforney at mforney.org
Sun Apr 26 11:32:07 AEST 2020
This matches the other do_convert_* functions, which also cannot
fail. Otherwise, ssh-keygen will crash when it tries to check the
key type before writing it to stdout.
For example, if I corrupt the key magic:
$ sed 's,^P2/5,AAAA,' regress/rsa_ssh2.prv > bad.prv && chmod 600 bad.prv
$ ssh-keygen -i -f bad.prv
bad magic 0xeb != 0x3f6ff9eb
Segmentation fault
$
---
ssh-keygen.c | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/ssh-keygen.c b/ssh-keygen.c
index d50ca5f2..6c160f0c 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -487,9 +487,8 @@ do_convert_private_ssh2(struct sshbuf *b)
fatal("%s: buffer error: %s", __func__, ssh_err(r));
if (magic != SSH_COM_PRIVATE_KEY_MAGIC) {
- error("bad magic 0x%x != 0x%x", magic,
+ fatal("bad magic 0x%x != 0x%x", magic,
SSH_COM_PRIVATE_KEY_MAGIC);
- return NULL;
}
if ((r = sshbuf_get_u32(b, &i1)) != 0 ||
(r = sshbuf_get_cstring(b, &type, NULL)) != 0 ||
@@ -499,12 +498,8 @@ do_convert_private_ssh2(struct sshbuf *b)
(r = sshbuf_get_u32(b, &i4)) != 0)
fatal("%s: buffer error: %s", __func__, ssh_err(r));
debug("ignore (%d %d %d %d)", i1, i2, i3, i4);
- if (strcmp(cipher, "none") != 0) {
- error("unsupported cipher %s", cipher);
- free(cipher);
- free(type);
- return NULL;
- }
+ if (strcmp(cipher, "none") != 0)
+ fatal("unsupported cipher %s", cipher);
free(cipher);
if (strstr(type, "dsa")) {
@@ -512,8 +507,7 @@ do_convert_private_ssh2(struct sshbuf *b)
} else if (strstr(type, "rsa")) {
ktype = KEY_RSA;
} else {
- free(type);
- return NULL;
+ fatal("unsupported key type %s", type);
}
if ((key = sshkey_new(ktype)) == NULL)
fatal("sshkey_new failed");
@@ -556,11 +550,8 @@ do_convert_private_ssh2(struct sshbuf *b)
}
if ((rsa_e = BN_new()) == NULL)
fatal("%s: BN_new", __func__);
- if (!BN_set_word(rsa_e, e)) {
- BN_clear_free(rsa_e);
- sshkey_free(key);
- return NULL;
- }
+ if (!BN_set_word(rsa_e, e))
+ fatal("%s: BN_set_word", __func__);
if ((rsa_n = BN_new()) == NULL ||
(rsa_d = BN_new()) == NULL ||
(rsa_p = BN_new()) == NULL ||
@@ -592,9 +583,7 @@ do_convert_private_ssh2(struct sshbuf *b)
NULL, NULL, 0) != 0 ||
sshkey_verify(key, sig, slen, data, sizeof(data),
NULL, 0, NULL) != 0) {
- sshkey_free(key);
- free(sig);
- return NULL;
+ fatal("%s: converted key is not valid", __func__);
}
free(sig);
return key;
--
2.26.2
More information about the openssh-unix-dev
mailing list