Skip to content

Commit

Permalink
Introduce a new utility function smemclr(), which memsets things to
Browse files Browse the repository at this point in the history 
zero but does it in such a way that over-clever compilers hopefully
won't helpfully optimise the call away if you do it just before
freeing something or letting it go out of scope. Use this for
(hopefully) every memset whose job is to destroy sensitive data that
might otherwise be left lying around in the process's memory.


git-svn-id: svn://svn.tartarus.org/sgt/putty@9586 cda61777-01e9-0310-a592-d414129be87e
  • Loading branch information
simon committed Jul 22, 2012
1 parent 7c3a733 commit ef97aa3
Show file tree
Hide file tree
Showing 21 changed files with 140 additions and 89 deletions.
6 changes: 3 additions & 3 deletions cmdgen.c
View file
Expand Up @@ -667,7 +667,7 @@ int main(int argc, char **argv)
return 1;
}
random_add_heavynoise(entropy, bits / 8);
memset(entropy, 0, bits/8);
smemclr(entropy, bits/8);
sfree(entropy);

if (keytype == DSA) {
Expand Down Expand Up @@ -860,7 +860,7 @@ int main(int argc, char **argv)
return 1;
}
if (passphrase) {
memset(passphrase, 0, strlen(passphrase));
smemclr(passphrase, strlen(passphrase));
sfree(passphrase);
}
passphrase = dupstr(p->prompts[0]->result);
Expand Down Expand Up @@ -1035,7 +1035,7 @@ int main(int argc, char **argv)
}

if (passphrase) {
memset(passphrase, 0, strlen(passphrase));
smemclr(passphrase, strlen(passphrase));
sfree(passphrase);
}

Expand Down
6 changes: 3 additions & 3 deletions cmdline.c
View file
Expand Up @@ -63,7 +63,7 @@ void cmdline_cleanup(void)
int pri;

if (cmdline_password) {
memset(cmdline_password, 0, strlen(cmdline_password));
smemclr(cmdline_password, strlen(cmdline_password));
sfree(cmdline_password);
cmdline_password = NULL;
}
Expand Down Expand Up @@ -106,7 +106,7 @@ int cmdline_get_passwd_input(prompts_t *p, unsigned char *in, int inlen) {
return 0;

prompt_set_result(p->prompts[0], cmdline_password);
memset(cmdline_password, 0, strlen(cmdline_password));
smemclr(cmdline_password, strlen(cmdline_password));
sfree(cmdline_password);
cmdline_password = NULL;
tried_once = 1;
Expand Down Expand Up @@ -368,7 +368,7 @@ int cmdline_process_param(char *p, char *value, int need_save, Conf *conf)
/* Assuming that `value' is directly from argv, make a good faith
* attempt to trample it, to stop it showing up in `ps' output
* on Unix-like systems. Not guaranteed, of course. */
memset(value, 0, strlen(value));
smemclr(value, strlen(value));
}
}

Expand Down
78 changes: 39 additions & 39 deletions import.c
View file
Expand Up @@ -358,7 +358,7 @@ static struct openssh_key *load_openssh_key(const Filename *filename,
errmsg = "unrecognised key type";
goto error;
}
memset(line, 0, strlen(line));
smemclr(line, strlen(line));
sfree(line);
line = NULL;

Expand Down Expand Up @@ -442,13 +442,13 @@ static struct openssh_key *load_openssh_key(const Filename *filename,
memcpy(ret->keyblob + ret->keyblob_len, out, len);
ret->keyblob_len += len;

memset(out, 0, sizeof(out));
smemclr(out, sizeof(out));
}

p++;
}
}
memset(line, 0, strlen(line));
smemclr(line, strlen(line));
sfree(line);
line = NULL;
}
Expand All @@ -463,23 +463,23 @@ static struct openssh_key *load_openssh_key(const Filename *filename,
goto error;
}

memset(base64_bit, 0, sizeof(base64_bit));
smemclr(base64_bit, sizeof(base64_bit));
if (errmsg_p) *errmsg_p = NULL;
return ret;

error:
if (line) {
memset(line, 0, strlen(line));
smemclr(line, strlen(line));
sfree(line);
line = NULL;
}
memset(base64_bit, 0, sizeof(base64_bit));
smemclr(base64_bit, sizeof(base64_bit));
if (ret) {
if (ret->keyblob) {
memset(ret->keyblob, 0, ret->keyblob_size);
smemclr(ret->keyblob, ret->keyblob_size);
sfree(ret->keyblob);
}
memset(ret, 0, sizeof(*ret));
smemclr(ret, sizeof(*ret));
sfree(ret);
}
if (errmsg_p) *errmsg_p = errmsg;
Expand All @@ -494,9 +494,9 @@ int openssh_encrypted(const Filename *filename)
if (!key)
return 0;
ret = key->encrypted;
memset(key->keyblob, 0, key->keyblob_size);
smemclr(key->keyblob, key->keyblob_size);
sfree(key->keyblob);
memset(key, 0, sizeof(*key));
smemclr(key, sizeof(*key));
sfree(key);
return ret;
}
Expand Down Expand Up @@ -564,8 +564,8 @@ struct ssh2_userkey *openssh_read(const Filename *filename, char *passphrase,
aes_free_context(ctx);
}

memset(&md5c, 0, sizeof(md5c));
memset(keybuf, 0, sizeof(keybuf));
smemclr(&md5c, sizeof(md5c));
smemclr(keybuf, sizeof(keybuf));
}

/*
Expand Down Expand Up @@ -698,12 +698,12 @@ struct ssh2_userkey *openssh_read(const Filename *filename, char *passphrase,

error:
if (blob) {
memset(blob, 0, blobsize);
smemclr(blob, blobsize);
sfree(blob);
}
memset(key->keyblob, 0, key->keyblob_size);
smemclr(key->keyblob, key->keyblob_size);
sfree(key->keyblob);
memset(key, 0, sizeof(*key));
smemclr(key, sizeof(*key));
sfree(key);
if (errmsg_p) *errmsg_p = errmsg;
return retval;
Expand Down Expand Up @@ -911,8 +911,8 @@ int openssh_write(const Filename *filename, struct ssh2_userkey *key,
*/
des3_encrypt_pubkey_ossh(keybuf, iv, outblob, outlen);

memset(&md5c, 0, sizeof(md5c));
memset(keybuf, 0, sizeof(keybuf));
smemclr(&md5c, sizeof(md5c));
smemclr(keybuf, sizeof(keybuf));
}

/*
Expand All @@ -936,19 +936,19 @@ int openssh_write(const Filename *filename, struct ssh2_userkey *key,

error:
if (outblob) {
memset(outblob, 0, outlen);
smemclr(outblob, outlen);
sfree(outblob);
}
if (spareblob) {
memset(spareblob, 0, sparelen);
smemclr(spareblob, sparelen);
sfree(spareblob);
}
if (privblob) {
memset(privblob, 0, privlen);
smemclr(privblob, privlen);
sfree(privblob);
}
if (pubblob) {
memset(pubblob, 0, publen);
smemclr(pubblob, publen);
sfree(pubblob);
}
return ret;
Expand Down Expand Up @@ -1067,7 +1067,7 @@ static struct sshcom_key *load_sshcom_key(const Filename *filename,
errmsg = "file does not begin with ssh.com key header";
goto error;
}
memset(line, 0, strlen(line));
smemclr(line, strlen(line));
sfree(line);
line = NULL;

Expand Down Expand Up @@ -1112,7 +1112,7 @@ static struct sshcom_key *load_sshcom_key(const Filename *filename,
len += line2len - 1;
assert(!line[len]);

memset(line2, 0, strlen(line2));
smemclr(line2, strlen(line2));
sfree(line2);
line2 = NULL;
}
Expand Down Expand Up @@ -1158,7 +1158,7 @@ static struct sshcom_key *load_sshcom_key(const Filename *filename,
p++;
}
}
memset(line, 0, strlen(line));
smemclr(line, strlen(line));
sfree(line);
line = NULL;
}
Expand All @@ -1173,16 +1173,16 @@ static struct sshcom_key *load_sshcom_key(const Filename *filename,

error:
if (line) {
memset(line, 0, strlen(line));
smemclr(line, strlen(line));
sfree(line);
line = NULL;
}
if (ret) {
if (ret->keyblob) {
memset(ret->keyblob, 0, ret->keyblob_size);
smemclr(ret->keyblob, ret->keyblob_size);
sfree(ret->keyblob);
}
memset(ret, 0, sizeof(*ret));
smemclr(ret, sizeof(*ret));
sfree(ret);
}
if (errmsg_p) *errmsg_p = errmsg;
Expand Down Expand Up @@ -1222,9 +1222,9 @@ int sshcom_encrypted(const Filename *filename, char **comment)

done:
*comment = dupstr(key->comment);
memset(key->keyblob, 0, key->keyblob_size);
smemclr(key->keyblob, key->keyblob_size);
sfree(key->keyblob);
memset(key, 0, sizeof(*key));
smemclr(key, sizeof(*key));
sfree(key);
return answer;
}
Expand Down Expand Up @@ -1390,8 +1390,8 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase,
des3_decrypt_pubkey_ossh(keybuf, iv, (unsigned char *)ciphertext,
cipherlen);

memset(&md5c, 0, sizeof(md5c));
memset(keybuf, 0, sizeof(keybuf));
smemclr(&md5c, sizeof(md5c));
smemclr(keybuf, sizeof(keybuf));

/*
* Hereafter we return WRONG_PASSPHRASE for any parsing
Expand Down Expand Up @@ -1494,12 +1494,12 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase,

error:
if (blob) {
memset(blob, 0, blobsize);
smemclr(blob, blobsize);
sfree(blob);
}
memset(key->keyblob, 0, key->keyblob_size);
smemclr(key->keyblob, key->keyblob_size);
sfree(key->keyblob);
memset(key, 0, sizeof(*key));
smemclr(key, sizeof(*key));
sfree(key);
if (errmsg_p) *errmsg_p = errmsg;
return ret;
Expand Down Expand Up @@ -1664,8 +1664,8 @@ int sshcom_write(const Filename *filename, struct ssh2_userkey *key,
des3_encrypt_pubkey_ossh(keybuf, iv, (unsigned char *)ciphertext,
cipherlen);

memset(&md5c, 0, sizeof(md5c));
memset(keybuf, 0, sizeof(keybuf));
smemclr(&md5c, sizeof(md5c));
smemclr(keybuf, sizeof(keybuf));
}

/*
Expand Down Expand Up @@ -1700,15 +1700,15 @@ int sshcom_write(const Filename *filename, struct ssh2_userkey *key,

error:
if (outblob) {
memset(outblob, 0, outlen);
smemclr(outblob, outlen);
sfree(outblob);
}
if (privblob) {
memset(privblob, 0, privlen);
smemclr(privblob, privlen);
sfree(privblob);
}
if (pubblob) {
memset(pubblob, 0, publen);
smemclr(pubblob, publen);
sfree(pubblob);
}
return ret;
Expand Down
46 changes: 43 additions & 3 deletions misc.c
View file
Expand Up @@ -124,7 +124,7 @@ void prompt_ensure_result_size(prompt_t *pr, int newlen)
*/
newbuf = snewn(newlen, char);
memcpy(newbuf, pr->result, pr->resultsize);
memset(pr->result, '\0', pr->resultsize);
smemclr(pr->result, pr->resultsize);
sfree(pr->result);
pr->result = newbuf;
pr->resultsize = newlen;
Expand All @@ -140,7 +140,7 @@ void free_prompts(prompts_t *p)
size_t i;
for (i=0; i < p->n_prompts; i++) {
prompt_t *pr = p->prompts[i];
memset(pr->result, 0, pr->resultsize); /* burn the evidence */
smemclr(pr->result, pr->resultsize); /* burn the evidence */
sfree(pr->result);
sfree(pr->prompt);
sfree(pr);
Expand Down Expand Up @@ -203,7 +203,7 @@ char *dupcat(const char *s1, ...)
void burnstr(char *string) /* sfree(str), only clear it first */
{
if (string) {
memset(string, 0, strlen(string));
smemclr(string, strlen(string));
sfree(string);
}
}
Expand Down Expand Up @@ -685,3 +685,43 @@ char const *conf_dest(Conf *conf)
else
return conf_get_str(conf, CONF_host);
}

#ifndef PLATFORM_HAS_SMEMCLR
/*
* Securely wipe memory.
*
* The actual wiping is no different from what memset would do: the
* point of 'securely' is to try to be sure over-clever compilers
* won't optimise away memsets on variables that are about to be freed
* or go out of scope. See
* https://buildsecurityin.us-cert.gov/bsi-rules/home/g1/771-BSI.html
*
* Some platforms (e.g. Windows) may provide their own version of this
* function.
*/
void smemclr(void *b, size_t n) {
volatile char *vp;

if (b && n > 0) {
/*
* Zero out the memory.
*/
memset(b, 0, n);

/*
* Perform a volatile access to the object, forcing the
* compiler to admit that the previous memset was important.
*
* This while loop should in practice run for zero iterations
* (since we know we just zeroed the object out), but in
* theory (as far as the compiler knows) it might range over
* the whole object. (If we had just written, say, '*vp =
* *vp;', a compiler could in principle have 'helpfully'
* optimised the memset into only zeroing out the first byte.
* This should be robust.)
*/
vp = b;
while (*vp) vp++;
}
}
#endif
1 change: 1 addition & 0 deletions putty.h
View file
Expand Up @@ -1291,6 +1291,7 @@ int filename_serialise(const Filename *f, void *data);
Filename *filename_deserialise(void *data, int maxsize, int *used);
char *get_username(void); /* return value needs freeing */
char *get_random_data(int bytes); /* used in cmdgen.c */
void smemclr(void *b, size_t len);

/*
* Exports and imports from timing.c.
Expand Down

0 comments on commit ef97aa3

Please sign in to comment.