From: Simon Tatham Date: Wed, 15 Feb 2017 05:31:30 +0000 (+0000) Subject: uxpgnt: correct control flow in find_key(). X-Git-Tag: 0.68~18 X-Git-Url: https://asedeno.scripts.mit.edu/gitweb/?a=commitdiff_plain;h=1266ac0e304b4e4db809eb045b8ac5dc43f299c4;p=PuTTY.git uxpgnt: correct control flow in find_key(). If we try to interpret a string argument as the name of a key file, sometimes we it's in circumstances where we _know_ it's a key file, so we must print an error message and return failure if the file can't be loaded. Other times it's not, and we just fall back to interpreting the argument in some other way (e.g. as a pattern match against the comment or fingerprint of a key already in the agent). My code dealing with failure returns from the public-key loading functions were mishandling the latter case, if they identified a file as existing and looking more or less like some kind of key file but then it turned out to have a format error; they would try to copy and return a public key that they didn't actually have. Even if pageant_pubkey_copy avoided crashing as a result, this would still inhibit the fallback to treating the input string as some other kind of pattern match. --- diff --git a/unix/uxpgnt.c b/unix/uxpgnt.c index a4bc6b93..50f65168 100644 --- a/unix/uxpgnt.c +++ b/unix/uxpgnt.c @@ -543,19 +543,19 @@ struct pageant_pubkey *find_key(const char *string, char **retstr) filename_free(fn); return NULL; } + } else { + /* + * If we've successfully loaded the file, stop here - we + * already have a key blob and need not go to the agent to + * list things. + */ + key_in.ssh_version = 1; + key_in.comment = NULL; + key_ret = pageant_pubkey_copy(&key_in); + sfree(key_in.blob); + filename_free(fn); + return key_ret; } - - /* - * If we've successfully loaded the file, stop here - we - * already have a key blob and need not go to the agent to - * list things. - */ - key_in.ssh_version = 1; - key_in.comment = NULL; - key_ret = pageant_pubkey_copy(&key_in); - sfree(key_in.blob); - filename_free(fn); - return key_ret; } else if (keytype == SSH_KEYTYPE_SSH2 || keytype == SSH_KEYTYPE_SSH2_PUBLIC_RFC4716 || keytype == SSH_KEYTYPE_SSH2_PUBLIC_OPENSSH) { @@ -570,19 +570,19 @@ struct pageant_pubkey *find_key(const char *string, char **retstr) filename_free(fn); return NULL; } + } else { + /* + * If we've successfully loaded the file, stop here - we + * already have a key blob and need not go to the agent to + * list things. + */ + key_in.ssh_version = 2; + key_in.comment = NULL; + key_ret = pageant_pubkey_copy(&key_in); + sfree(key_in.blob); + filename_free(fn); + return key_ret; } - - /* - * If we've successfully loaded the file, stop here - we - * already have a key blob and need not go to the agent to - * list things. - */ - key_in.ssh_version = 2; - key_in.comment = NULL; - key_ret = pageant_pubkey_copy(&key_in); - sfree(key_in.blob); - filename_free(fn); - return key_ret; } else { if (file_errors) { *retstr = dupprintf("unable to load key file '%s': %s",