~mdw
/
sgt
/
putty
/ blobdiff
commit
grep
author
committer
pickaxe
?
search:
re
summary
|
shortlog
|
log
|
commit
|
commitdiff
|
tree
raw
|
inline
| side by side
Another big batch of memory leak fixes, again mostly on error paths.
[sgt/putty]
/
windows
/
winpgnt.c
diff --git
a/windows/winpgnt.c
b/windows/winpgnt.c
index
9d59327
..
1f6484f
100644
(file)
--- a/
windows/winpgnt.c
+++ b/
windows/winpgnt.c
@@
-159,10
+159,8
@@
struct blob {
};
static int cmpkeys_ssh2_asymm(void *av, void *bv);
};
static int cmpkeys_ssh2_asymm(void *av, void *bv);
-#define PASSPHRASE_MAXLEN 512
-
struct PassphraseProcStruct {
struct PassphraseProcStruct {
- char *passphrase;
+ char *
*
passphrase;
char *comment;
};
char *comment;
};
@@
-176,7
+174,7
@@
static void forget_passphrases(void)
{
while (count234(passphrases) > 0) {
char *pp = index234(passphrases, 0);
{
while (count234(passphrases) > 0) {
char *pp = index234(passphrases, 0);
-
memset(pp, 0
, strlen(pp));
+
smemclr(pp
, strlen(pp));
delpos234(passphrases, 0);
free(pp);
}
delpos234(passphrases, 0);
free(pp);
}
@@
-247,7
+245,7
@@
static HWND passphrase_box;
static int CALLBACK PassphraseProc(HWND hwnd, UINT msg,
WPARAM wParam, LPARAM lParam)
{
static int CALLBACK PassphraseProc(HWND hwnd, UINT msg,
WPARAM wParam, LPARAM lParam)
{
- static char *passphrase = NULL;
+ static char *
*
passphrase = NULL;
struct PassphraseProcStruct *p;
switch (msg) {
struct PassphraseProcStruct *p;
switch (msg) {
@@
-275,8
+273,9
@@
static int CALLBACK PassphraseProc(HWND hwnd, UINT msg,
passphrase = p->passphrase;
if (p->comment)
SetDlgItemText(hwnd, 101, p->comment);
passphrase = p->passphrase;
if (p->comment)
SetDlgItemText(hwnd, 101, p->comment);
- *passphrase = 0;
- SetDlgItemText(hwnd, 102, passphrase);
+ burnstr(*passphrase);
+ *passphrase = dupstr("");
+ SetDlgItemText(hwnd, 102, *passphrase);
return 0;
case WM_COMMAND:
switch (LOWORD(wParam)) {
return 0;
case WM_COMMAND:
switch (LOWORD(wParam)) {
@@
-291,9
+290,8
@@
static int CALLBACK PassphraseProc(HWND hwnd, UINT msg,
return 0;
case 102: /* edit box */
if ((HIWORD(wParam) == EN_CHANGE) && passphrase) {
return 0;
case 102: /* edit box */
if ((HIWORD(wParam) == EN_CHANGE) && passphrase) {
- GetDlgItemText(hwnd, 102, passphrase,
- PASSPHRASE_MAXLEN - 1);
- passphrase[PASSPHRASE_MAXLEN - 1] = '\0';
+ burnstr(*passphrase);
+ *passphrase = GetDlgItemText_alloc(hwnd, 102);
}
return 0;
}
}
return 0;
}
@@
-355,28
+353,27
@@
static void keylist_update(void)
0, (LPARAM) listentry);
}
for (i = 0; NULL != (skey = index234(ssh2keys, i)); i++) {
0, (LPARAM) listentry);
}
for (i = 0; NULL != (skey = index234(ssh2keys, i)); i++) {
- char
listentry[512]
, *p;
- int len;
+ char
*listentry
, *p;
+ int
fp_
len;
/*
* Replace two spaces in the fingerprint with tabs, for
* nice alignment in the box.
*/
p = skey->alg->fingerprint(skey->data);
/*
* Replace two spaces in the fingerprint with tabs, for
* nice alignment in the box.
*/
p = skey->alg->fingerprint(skey->data);
- strncpy(listentry, p, sizeof(listentry));
+ listentry = dupprintf("%s\t%s", p, skey->comment);
+ fp_len = strlen(listentry);
+ sfree(p);
+
p = strchr(listentry, ' ');
p = strchr(listentry, ' ');
- if (p)
+ if (p
&& p < listentry + fp_len
)
*p = '\t';
p = strchr(listentry, ' ');
*p = '\t';
p = strchr(listentry, ' ');
- if (p)
+ if (p
&& p < listentry + fp_len
)
*p = '\t';
*p = '\t';
- len = strlen(listentry);
- if (len < sizeof(listentry) - 2) {
- listentry[len] = '\t';
- strncpy(listentry + len + 1, skey->comment,
- sizeof(listentry) - len - 1);
- }
+
SendDlgItemMessage(keylist, 100, LB_ADDSTRING, 0,
(LPARAM) listentry);
SendDlgItemMessage(keylist, 100, LB_ADDSTRING, 0,
(LPARAM) listentry);
+ sfree(listentry);
}
SendDlgItemMessage(keylist, 100, LB_SETCURSEL, (WPARAM) - 1, 0);
}
}
SendDlgItemMessage(keylist, 100, LB_SETCURSEL, (WPARAM) - 1, 0);
}
@@
-387,7
+384,7
@@
static void keylist_update(void)
*/
static void add_keyfile(Filename *filename)
{
*/
static void add_keyfile(Filename *filename)
{
- char
passphrase[PASSPHRASE_MAXLEN]
;
+ char
*passphrase
;
struct RSAKey *rkey = NULL;
struct ssh2_userkey *skey = NULL;
int needs_pass;
struct RSAKey *rkey = NULL;
struct ssh2_userkey *skey = NULL;
int needs_pass;
@@
-395,7
+392,6
@@
static void add_keyfile(Filename *filename)
int attempts;
char *comment;
const char *error = NULL;
int attempts;
char *comment;
const char *error = NULL;
- struct PassphraseProcStruct pps;
int type;
int original_pass;
int type;
int original_pass;
@@
-453,7
+449,12
@@
static void add_keyfile(Filename *filename)
MB_OK | MB_ICONERROR);
return;
}
MB_OK | MB_ICONERROR);
return;
}
- nkeys = GET_32BIT(keylist);
+ nkeys = toint(GET_32BIT(keylist));
+ if (nkeys < 0) {
+ MessageBox(NULL, "Received broken key list?!", APPNAME,
+ MB_OK | MB_ICONERROR);
+ return;
+ }
p = keylist + 4;
keylistlen -= 4;
p = keylist + 4;
keylistlen -= 4;
@@
-481,8
+482,8
@@
static void add_keyfile(Filename *filename)
MB_OK | MB_ICONERROR);
return;
}
MB_OK | MB_ICONERROR);
return;
}
- n =
4 + GET_32BIT(p
);
- if (keylistlen < n) {
+ n =
toint(4 + GET_32BIT(p)
);
+ if (
n < 0 ||
keylistlen < n) {
MessageBox(NULL, "Received broken key list?!", APPNAME,
MB_OK | MB_ICONERROR);
return;
MessageBox(NULL, "Received broken key list?!", APPNAME,
MB_OK | MB_ICONERROR);
return;
@@
-498,8
+499,8
@@
static void add_keyfile(Filename *filename)
MB_OK | MB_ICONERROR);
return;
}
MB_OK | MB_ICONERROR);
return;
}
- n =
4 + GET_32BIT(p
);
- if (keylistlen < n) {
+ n =
toint(4 + GET_32BIT(p)
);
+ if (
n < 0 ||
keylistlen < n) {
MessageBox(NULL, "Received broken key list?!", APPNAME,
MB_OK | MB_ICONERROR);
return;
MessageBox(NULL, "Received broken key list?!", APPNAME,
MB_OK | MB_ICONERROR);
return;
@@
-523,17
+524,24
@@
static void add_keyfile(Filename *filename)
attempts = 0;
if (type == SSH_KEYTYPE_SSH1)
rkey = snew(struct RSAKey);
attempts = 0;
if (type == SSH_KEYTYPE_SSH1)
rkey = snew(struct RSAKey);
- pps.passphrase = passphrase;
- pps.comment = comment;
+ passphrase = NULL;
original_pass = 0;
do {
original_pass = 0;
do {
+ burnstr(passphrase);
+ passphrase = NULL;
+
if (needs_pass) {
/* try all the remembered passphrases first */
char *pp = index234(passphrases, attempts);
if(pp) {
if (needs_pass) {
/* try all the remembered passphrases first */
char *pp = index234(passphrases, attempts);
if(pp) {
-
strcpy(passphrase,
pp);
+
passphrase = dupstr(
pp);
} else {
int dlgret;
} else {
int dlgret;
+ struct PassphraseProcStruct pps;
+
+ pps.passphrase = &passphrase;
+ pps.comment = comment;
+
original_pass = 1;
dlgret = DialogBoxParam(hinst, MAKEINTRESOURCE(210),
NULL, PassphraseProc, (LPARAM) &pps);
original_pass = 1;
dlgret = DialogBoxParam(hinst, MAKEINTRESOURCE(210),
NULL, PassphraseProc, (LPARAM) &pps);
@@
-545,9
+553,12
@@
static void add_keyfile(Filename *filename)
sfree(rkey);
return; /* operation cancelled */
}
sfree(rkey);
return; /* operation cancelled */
}
+
+ assert(passphrase != NULL);
}
} else
}
} else
- *passphrase = '\0';
+ passphrase = dupstr("");
+
if (type == SSH_KEYTYPE_SSH1)
ret = loadrsakey(filename, rkey, passphrase, &error);
else {
if (type == SSH_KEYTYPE_SSH1)
ret = loadrsakey(filename, rkey, passphrase, &error);
else {
@@
-562,11
+573,14
@@
static void add_keyfile(Filename *filename)
attempts++;
} while (ret == -1);
attempts++;
} while (ret == -1);
- /* if they typed in an ok passphrase, remember it */
if(original_pass && ret) {
if(original_pass && ret) {
- char *pp = dupstr(passphrase);
- addpos234(passphrases, pp, 0);
+ /* If they typed in an ok passphrase, remember it */
+ addpos234(passphrases, passphrase, 0);
+ } else {
+ /* Otherwise, destroy it */
+ burnstr(passphrase);
}
}
+ passphrase = NULL;
if (comment)
sfree(comment);
if (comment)
sfree(comment);
@@
-797,8
+811,10
@@
static void *get_keylist1(int *length)
retval = agent_query(request, 5, &vresponse, &resplen, NULL, NULL);
assert(retval == 1);
response = vresponse;
retval = agent_query(request, 5, &vresponse, &resplen, NULL, NULL);
assert(retval == 1);
response = vresponse;
- if (resplen < 5 || response[4] != SSH1_AGENT_RSA_IDENTITIES_ANSWER)
+ if (resplen < 5 || response[4] != SSH1_AGENT_RSA_IDENTITIES_ANSWER) {
+ sfree(response);
return NULL;
return NULL;
+ }
ret = snewn(resplen-5, unsigned char);
memcpy(ret, response+5, resplen-5);
ret = snewn(resplen-5, unsigned char);
memcpy(ret, response+5, resplen-5);
@@
-832,8
+848,10
@@
static void *get_keylist2(int *length)
retval = agent_query(request, 5, &vresponse, &resplen, NULL, NULL);
assert(retval == 1);
response = vresponse;
retval = agent_query(request, 5, &vresponse, &resplen, NULL, NULL);
assert(retval == 1);
response = vresponse;
- if (resplen < 5 || response[4] != SSH2_AGENT_IDENTITIES_ANSWER)
+ if (resplen < 5 || response[4] != SSH2_AGENT_IDENTITIES_ANSWER) {
+ sfree(response);
return NULL;
return NULL;
+ }
ret = snewn(resplen-5, unsigned char);
memcpy(ret, response+5, resplen-5);
ret = snewn(resplen-5, unsigned char);
memcpy(ret, response+5, resplen-5);
@@
-928,12
+946,17
@@
static void answer_msg(void *msg)
goto failure;
p += i;
i = ssh1_read_bignum(p, msgend - p, &reqkey.modulus);
goto failure;
p += i;
i = ssh1_read_bignum(p, msgend - p, &reqkey.modulus);
- if (i < 0)
+ if (i < 0) {
+ freebn(reqkey.exponent);
goto failure;
goto failure;
+ }
p += i;
i = ssh1_read_bignum(p, msgend - p, &challenge);
p += i;
i = ssh1_read_bignum(p, msgend - p, &challenge);
- if (i < 0)
+ if (i < 0) {
+ freebn(reqkey.exponent);
+ freebn(reqkey.modulus);
goto failure;
goto failure;
+ }
p += i;
if (msgend < p+16) {
freebn(reqkey.exponent);
p += i;
if (msgend < p+16) {
freebn(reqkey.exponent);
@@
-958,7
+981,7
@@
static void answer_msg(void *msg)
MD5Init(&md5c);
MD5Update(&md5c, response_source, 48);
MD5Final(response_md5, &md5c);
MD5Init(&md5c);
MD5Update(&md5c, response_source, 48);
MD5Final(response_md5, &md5c);
-
memset(response_source, 0, 48);
/* burn the evidence */
+
smemclr(response_source, 48);
/* burn the evidence */
freebn(response); /* and that evidence */
freebn(challenge); /* yes, and that evidence */
freebn(reqkey.exponent); /* and free some memory ... */
freebn(response); /* and that evidence */
freebn(challenge); /* yes, and that evidence */
freebn(reqkey.exponent); /* and free some memory ... */
@@
-988,17
+1011,17
@@
static void answer_msg(void *msg)
if (msgend < p+4)
goto failure;
if (msgend < p+4)
goto failure;
- b.len = GET_32BIT(p);
+ b.len = toint(GET_32BIT(p));
+ if (b.len < 0 || b.len > msgend - (p+4))
+ goto failure;
p += 4;
p += 4;
- if (msgend < p+b.len)
- goto failure;
b.blob = p;
p += b.len;
if (msgend < p+4)
goto failure;
b.blob = p;
p += b.len;
if (msgend < p+4)
goto failure;
- datalen =
GET_32BIT(p
);
+ datalen =
toint(GET_32BIT(p)
);
p += 4;
p += 4;
- if (
msgend < p+datalen
)
+ if (
datalen < 0 || datalen > msgend - p
)
goto failure;
data = p;
key = find234(ssh2keys, &b, cmpkeys_ssh2_asymm);
goto failure;
data = p;
key = find234(ssh2keys, &b, cmpkeys_ssh2_asymm);
@@
-1071,9
+1094,9
@@
static void answer_msg(void *msg)
sfree(key);
goto failure;
}
sfree(key);
goto failure;
}
- commentlen =
GET_32BIT(p
);
+ commentlen =
toint(GET_32BIT(p)
);
- if (
msgend < p+commentlen
) {
+ if (
commentlen < 0 || commentlen > msgend - p
) {
freersakey(key);
sfree(key);
goto failure;
freersakey(key);
sfree(key);
goto failure;
@@
-1110,9
+1133,9
@@
static void answer_msg(void *msg)
if (msgend < p+4)
goto failure;
if (msgend < p+4)
goto failure;
- alglen =
GET_32BIT(p
);
+ alglen =
toint(GET_32BIT(p)
);
p += 4;
p += 4;
- if (
msgend < p+alglen
)
+ if (
alglen < 0 || alglen > msgend - p
)
goto failure;
alg = p;
p += alglen;
goto failure;
alg = p;
p += alglen;
@@
-1146,10
+1169,10
@@
static void answer_msg(void *msg)
sfree(key);
goto failure;
}
sfree(key);
goto failure;
}
- commlen =
GET_32BIT(p
);
+ commlen =
toint(GET_32BIT(p)
);
p += 4;
p += 4;
- if (
msgend < p+commlen
) {
+ if (
commlen < 0 || commlen > msgend - p
) {
key->alg->freekey(key->data);
sfree(key);
goto failure;
key->alg->freekey(key->data);
sfree(key);
goto failure;
@@
-1213,10
+1236,10
@@
static void answer_msg(void *msg)
if (msgend < p+4)
goto failure;
if (msgend < p+4)
goto failure;
- b.len =
GET_32BIT(p
);
+ b.len =
toint(GET_32BIT(p)
);
p += 4;
p += 4;
- if (
msgend < p+b.len
)
+ if (
b.len < 0 || b.len > msgend - p
)
goto failure;
b.blob = p;
p += b.len;
goto failure;
b.blob = p;
p += b.len;
@@
-1423,10
+1446,12
@@
static void prompt_add_keyfile(void)
of.lpstrTitle = "Select Private Key File";
of.Flags = OFN_ALLOWMULTISELECT | OFN_EXPLORER;
if (request_file(keypath, &of, TRUE, FALSE)) {
of.lpstrTitle = "Select Private Key File";
of.Flags = OFN_ALLOWMULTISELECT | OFN_EXPLORER;
if (request_file(keypath, &of, TRUE, FALSE)) {
- if(strlen(filelist) > of.nFileOffset)
+ if(strlen(filelist) > of.nFileOffset)
{
/* Only one filename returned? */
/* Only one filename returned? */
- add_keyfile(filename_from_str(filelist));
- else {
+ Filename *fn = filename_from_str(filelist);
+ add_keyfile(fn);
+ filename_free(fn);
+ } else {
/* we are returned a bunch of strings, end to
* end. first string is the directory, the
* rest the filenames. terminated with an
/* we are returned a bunch of strings, end to
* end. first string is the directory, the
* rest the filenames. terminated with an
@@
-1436,7
+1461,9
@@
static void prompt_add_keyfile(void)
char *filewalker = filelist + strlen(dir) + 1;
while (*filewalker != '\0') {
char *filename = dupcat(dir, "\\", filewalker, NULL);
char *filewalker = filelist + strlen(dir) + 1;
while (*filewalker != '\0') {
char *filename = dupcat(dir, "\\", filewalker, NULL);
- add_keyfile(filename_from_str(filename));
+ Filename *fn = filename_from_str(filename);
+ add_keyfile(fn);
+ filename_free(fn);
sfree(filename);
filewalker += strlen(filewalker) + 1;
}
sfree(filename);
filewalker += strlen(filewalker) + 1;
}
@@
-1894,6
+1921,7
@@
static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
#ifdef DEBUG_IPC
debug(("couldn't get user SID\n"));
#endif
#ifdef DEBUG_IPC
debug(("couldn't get user SID\n"));
#endif
+ CloseHandle(filemap);
return 0;
}
return 0;
}
@@
-1901,6
+1929,8
@@
static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
#ifdef DEBUG_IPC
debug(("couldn't get default SID\n"));
#endif
#ifdef DEBUG_IPC
debug(("couldn't get default SID\n"));
#endif
+ CloseHandle(filemap);
+ sfree(ourself);
return 0;
}
return 0;
}
@@
-1912,6
+1942,9
@@
static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
debug(("couldn't get owner info for filemap: %d\n",
rc));
#endif
debug(("couldn't get owner info for filemap: %d\n",
rc));
#endif
+ CloseHandle(filemap);
+ sfree(ourself);
+ sfree(ourself2);
return 0;
}
#ifdef DEBUG_IPC
return 0;
}
#ifdef DEBUG_IPC
@@
-1928,8
+1961,13
@@
static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
}
#endif
if (!EqualSid(mapowner, ourself) &&
}
#endif
if (!EqualSid(mapowner, ourself) &&
- !EqualSid(mapowner, ourself2))
+ !EqualSid(mapowner, ourself2)) {
+ CloseHandle(filemap);
+ LocalFree(psd);
+ sfree(ourself);
+ sfree(ourself2);
return 0; /* security ID mismatch! */
return 0; /* security ID mismatch! */
+ }
#ifdef DEBUG_IPC
debug(("security stuff matched\n"));
#endif
#ifdef DEBUG_IPC
debug(("security stuff matched\n"));
#endif
@@
-2111,7
+2149,9
@@
int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show)
command = "";
break;
} else {
command = "";
break;
} else {
- add_keyfile(filename_from_str(argv[i]));
+ Filename *fn = filename_from_str(argv[i]);
+ add_keyfile(fn);
+ filename_free(fn);
added_keys = TRUE;
}
}
added_keys = TRUE;
}
}