CVE-2011-4607: PuTTY Password-not-Wiped Vulnerability

This was a very interesting vulnerability disclosed by the PuTTY project through this security advisory.

The buggy code resides in putty/ssh.c file and more specifically in the C routine you see here.

/*
 * Handle the SSH-2 userauth and connection layers.
 */
static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
			     struct Packet *pktin)
{
    struct do_ssh2_authconn_state {
	enum {
	    AUTH_TYPE_NONE,
		AUTH_TYPE_PUBLICKEY,
		AUTH_TYPE_PUBLICKEY_OFFER_LOUD,
		AUTH_TYPE_PUBLICKEY_OFFER_QUIET,
		AUTH_TYPE_PASSWORD,
	        AUTH_TYPE_GSSAPI,      /* always QUIET */
		AUTH_TYPE_KEYBOARD_INTERACTIVE,
		AUTH_TYPE_KEYBOARD_INTERACTIVE_QUIET
	} type;
	int done_service_req;
	int gotit, need_pw, can_pubkey, can_passwd, can_keyb_inter;
	int tried_pubkey_config, done_agent;
#ifndef NO_GSSAPI
	int can_gssapi;
	int tried_gssapi;
#endif
	int kbd_inter_refused;
	int we_are_in, userauth_success;
	prompts_t *cur_prompt;
	int num_prompts;
	char *username;
	char *password;
	int got_username;
	void *publickey_blob;
	int publickey_bloblen;
	int publickey_encrypted;
	char *publickey_algorithm;
	char *publickey_comment;
	unsigned char agent_request[5], *agent_response, *agentp;
	int agent_responselen;
	unsigned char *pkblob_in_agent;
	int keyi, nkeys;
	char *pkblob, *alg, *commentp;
	int pklen, alglen, commentlen;
	int siglen, retlen, len;
	char *q, *agentreq, *ret;
	int try_send;
	int num_env, env_left, env_ok;
	struct Packet *pktout;
	Filename *keyfile;
#ifndef NO_GSSAPI
	struct ssh_gss_library *gsslib;
	Ssh_gss_ctx gss_ctx;
	Ssh_gss_buf gss_buf;
	Ssh_gss_buf gss_rcvtok, gss_sndtok;
	Ssh_gss_name gss_srv_name;
	Ssh_gss_stat gss_stat;
#endif
    };
    crState(do_ssh2_authconn_state);

    crBegin(ssh->do_ssh2_authconn_crstate);

    s->done_service_req = FALSE;
    s->we_are_in = s->userauth_success = FALSE;
#ifndef NO_GSSAPI
    s->tried_gssapi = FALSE;
#endif

    if (!conf_get_int(ssh->conf, CONF_ssh_no_userauth)) {
  ...
    crFinishV;
}

This is a huge function which uses a ‘Socket’ structure which also includes a member named ‘cur_prompt’ of type ‘prompts_t’. This type is defined in putty/putty.h header file as shown below.

/*
 * Mechanism for getting text strings such as usernames and passwords
 * from the front-end.
 * The fields are mostly modelled after SSH's keyboard-interactive auth.
 * FIXME We should probably mandate a character set/encoding (probably UTF-8).
 *
 * Since many of the pieces of text involved may be chosen by the server,
 * the caller must take care to ensure that the server can't spoof locally-
 * generated prompts such as key passphrase prompts. Some ground rules:
 *  - If the front-end needs to truncate a string, it should lop off the
 *    end.
 *  - The front-end should filter out any dangerous characters and
 *    generally not trust the strings. (But \n is required to behave
 *    vaguely sensibly, at least in `instruction', and ideally in
 *    `prompt[]' too.)
 */
typedef struct {
    char *prompt;
    int echo;
    /*
     * 'result' must be a dynamically allocated array of exactly
     * 'resultsize' chars. The code for actually reading input may
     * realloc it bigger (and adjust resultsize accordingly) if it has
     * to. The caller should free it again when finished with it.
     *
     * If resultsize==0, then result may be NULL. When setting up a
     * prompt_t, it's therefore easiest to initialise them this way,
     * which means all actual allocation is done by the callee. This
     * is what add_prompt does.
     */
    char *result;
    size_t resultsize;
} prompt_t;
typedef struct {
    /*
     * Indicates whether the information entered is to be used locally
     * (for instance a key passphrase prompt), or is destined for the wire.
     * This is a hint only; the front-end is at liberty not to use this
     * information (so the caller should ensure that the supplied text is
     * sufficient).
     */
    int to_server;
    char *name;		/* Short description, perhaps for dialog box title */
    int name_reqd;	/* Display of `name' required or optional? */
    char *instruction;	/* Long description, maybe with embedded newlines */
    int instr_reqd;	/* Display of `instruction' required or optional? */
    size_t n_prompts;   /* May be zero (in which case display the foregoing,
                         * if any, and return success) */
    prompt_t **prompts;
    void *frontend;
    void *data;		/* slot for housekeeping data, managed by
			 * get_userpass_input(); initially NULL */
} prompts_t;
prompts_t *new_prompts(void *frontend);
void add_prompt(prompts_t *p, char *promptstr, int echo);
void prompt_set_result(prompt_t *pr, const char *newstr);
void prompt_ensure_result_size(prompt_t *pr, int len);
/* Burn the evidence. (Assumes _all_ strings want free()ing.) */
void free_prompts(prompts_t *p);

The problem with the initial routine was that it was not using free_prompts() to “burn the evidence” as the above code comment suggests. Due to this mistake, critical data such as passwords and usernames were not erased from memory and a user able to read PuTTY process’ memory could retrieve those data.

The fix was to add the missing call like this:

 		    ssh2_pkt_send_with_padding(ssh, s->pktout, 256);
 
+                    /*
+                     * Free the prompts structure from this iteration.
+                     * If there's another, a new one will be allocated
+                     * when we return to the top of this while loop.
+                     */
+                    free_prompts(s->cur_prompt);
+
 		    /*
 		     * Get the next packet in case it's another
 		     * INFO_REQUEST.

About these ads