From d53b3bcd22ace9062e0be66c28f868dcc77c2b34 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Fri, 9 Apr 2021 18:21:40 +0100 Subject: [PATCH] pageant_get_keylist: fix handling of bad SSH-1 data from agent. Coverity points out that if rsa_ssh1_public_blob_len sees data it doesn't like, it returns -1 to indicate an error. But the code that uses it to parse the SSH1_AGENT_RSA_IDENTITIES_ANSWER payload was passing it directly to get_data() as a length field, without checking for that case. Now we do check it, and use it to set the existing kl->broken flag that indicates that the key list was not correctly formatted. --- pageant.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/pageant.c b/pageant.c index 699cd579..5d7e0b8d 100644 --- a/pageant.c +++ b/pageant.c @@ -1897,8 +1897,13 @@ static KeyList *pageant_get_keylist(unsigned ssh_version) for (size_t i = 0; i < kl->nkeys && !get_err(pco); i++) { if (ssh_version == 1) { - kl->keys[i].blob = get_data(pco, rsa_ssh1_public_blob_len( - make_ptrlen(get_ptr(pco), get_avail(pco)))); + int bloblen = rsa_ssh1_public_blob_len( + make_ptrlen(get_ptr(pco), get_avail(pco))); + if (bloblen < 0) { + kl->broken = true; + bloblen = 0; + } + kl->keys[i].blob = get_data(pco, bloblen); } else { kl->keys[i].blob = get_string(pco); } @@ -1915,7 +1920,8 @@ static KeyList *pageant_get_keylist(unsigned ssh_version) } } - kl->broken = get_err(pco); + if (get_err(pco)) + kl->broken = true; kl->raw_data = pco->buf; pco->buf = NULL; pageant_client_op_free(pco);