From 7c95ea19c88fc7a547184ed84276fb3a6e2a5ba1 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Thu, 7 Nov 2002 19:49:03 +0000 Subject: [PATCH] Robustness fixes for KEXINIT handling and others. In particular, I've created a self-mallocing variant of sprintf, to obviate any future need for paranoid %.100s type stuff in format strings. [originally from svn r2199] --- misc.c | 60 +++++++++++++- misc.h | 8 +- pageant.c | 16 ++-- proxy.c | 13 +-- psftp.c | 33 ++++---- puttygen.c | 25 +++--- raw.c | 10 ++- rlogin.c | 10 ++- scp.c | 89 ++++++++++---------- ssh.c | 239 ++++++++++++++++++++++++----------------------------- telnet.c | 50 ++++++----- 11 files changed, 306 insertions(+), 247 deletions(-) diff --git a/misc.c b/misc.c index 2d0a4634..6b2f9e68 100644 --- a/misc.c +++ b/misc.c @@ -1,6 +1,7 @@ #include #include #include +#include #include #include "putty.h" @@ -8,7 +9,7 @@ * String handling routines. */ -char *dupstr(char *s) +char *dupstr(const char *s) { int len = strlen(s); char *p = smalloc(len + 1); @@ -17,7 +18,7 @@ char *dupstr(char *s) } /* Allocate the concatenation of N strings. Terminate arg list with NULL. */ -char *dupcat(char *s1, ...) +char *dupcat(const char *s1, ...) { int len; char *p, *q, *sn; @@ -50,6 +51,56 @@ char *dupcat(char *s1, ...) return p; } +/* + * Do an sprintf(), but into a custom-allocated buffer. + * + * Irritatingly, we don't seem to be able to do this portably using + * vsnprintf(), because there appear to be issues with re-using the + * same va_list for two calls, and the excellent C99 va_copy is not + * yet widespread. Bah. Instead I'm going to do a horrid, horrid + * hack, in which I trawl the format string myself, work out the + * maximum length of each format component, and resize the buffer + * before printing it. + */ +char *dupprintf(const char *fmt, ...) +{ + char *ret; + va_list ap; + va_start(ap, fmt); + ret = dupvprintf(fmt, ap); + va_end(ap); + return ret; +} +char *dupvprintf(const char *fmt, va_list ap) +{ + char *buf; + int len, size; + + buf = smalloc(512); + size = 512; + + while (1) { +#ifdef _WINDOWS +#define vsnprintf _vsnprintf +#endif + len = vsnprintf(buf, size, fmt, ap); + if (len >= 0 && len < size) { + /* This is the C99-specified criterion for snprintf to have + * been completely successful. */ + return buf; + } else if (len > 0) { + /* This is the C99 error condition: the returned length is + * the required buffer size not counting the NUL. */ + size = len + 1; + } else { + /* This is the pre-C99 glibc error condition: <0 means the + * buffer wasn't big enough, so we enlarge it a bit and hope. */ + size += 512; + } + buf = srealloc(buf, size); + } +} + /* ---------------------------------------------------------------------- * Base64 encoding routine. This is required in public-key writing * but also in HTTP proxy handling, so it's centralised here. @@ -564,12 +615,13 @@ static void dputs(char *buf) void dprintf(char *fmt, ...) { - char buf[2048]; + char *buf; va_list ap; va_start(ap, fmt); - vsprintf(buf, fmt, ap); + buf = dupvprintf(fmt, ap); dputs(buf); + sfree(buf); va_end(ap); } diff --git a/misc.h b/misc.h index 48054745..21540a3b 100644 --- a/misc.h +++ b/misc.h @@ -3,6 +3,8 @@ #include "puttymem.h" +#include /* for va_list */ + #ifndef FALSE #define FALSE 0 #endif @@ -10,8 +12,10 @@ #define TRUE 1 #endif -char *dupstr(char *s); -char *dupcat(char *s1, ...); +char *dupstr(const char *s); +char *dupcat(const char *s1, ...); +char *dupprintf(const char *fmt, ...); +char *dupvprintf(const char *fmt, va_list ap); void base64_encode_atom(unsigned char *data, int n, char *out); diff --git a/pageant.c b/pageant.c index b8a4bd81..0ffb24c2 100644 --- a/pageant.c +++ b/pageant.c @@ -65,13 +65,14 @@ static int initial_menuitems_count; void modalfatalbox(char *fmt, ...) { va_list ap; - char stuff[200]; + char *buf; va_start(ap, fmt); - vsprintf(stuff, fmt, ap); + buf = dupvprintf(fmt, ap); va_end(ap); - MessageBox(main_hwnd, stuff, "Pageant Fatal Error", + MessageBox(main_hwnd, buf, "Pageant Fatal Error", MB_SYSTEMMODAL | MB_ICONERROR | MB_OK); + sfree(buf); exit(1); } @@ -1781,10 +1782,11 @@ void spawn_cmd(char *cmdline, char * args, int show) { if (ShellExecute(NULL, _T("open"), cmdline, args, NULL, show) <= (HINSTANCE) 32) { - TCHAR sMsg[140]; - sprintf(sMsg, _T("Failed to run \"%.100s\", Error: %d"), cmdline, - (int)GetLastError()); - MessageBox(NULL, sMsg, APPNAME, MB_OK | MB_ICONEXCLAMATION); + char *msg; + msg = dupprintf("Failed to run \"%.100s\", Error: %d", cmdline, + (int)GetLastError()); + MessageBox(NULL, msg, APPNAME, MB_OK | MB_ICONEXCLAMATION); + sfree(msg); } } diff --git a/proxy.c b/proxy.c index 0760e7e8..a02fc11c 100644 --- a/proxy.c +++ b/proxy.c @@ -459,13 +459,14 @@ int proxy_http_negotiate (Proxy_Socket p, int change) * for this proxy method, it's just a simple HTTP * request */ - char buf[256], dest[64]; + char *buf, dest[64]; sk_getaddr(p->remote_addr, dest, 64); - sprintf(buf, "CONNECT %s:%i HTTP/1.1\r\nHost: %s:%i\r\n", - dest, p->remote_port, dest, p->remote_port); + buf = dupprintf("CONNECT %s:%i HTTP/1.1\r\nHost: %s:%i\r\n", + dest, p->remote_port, dest, p->remote_port); sk_write(p->sub_socket, buf, strlen(buf)); + sfree(buf); if (cfg.proxy_username[0] || cfg.proxy_password[0]) { char buf[sizeof(cfg.proxy_username)+sizeof(cfg.proxy_password)]; @@ -556,14 +557,14 @@ int proxy_http_negotiate (Proxy_Socket p, int change) bufchain_consume(&p->pending_input_data, eol); if (data[status] != '2') { /* error */ - char buf[1024]; + char *buf; data[eol] = '\0'; while (eol > status && (data[eol-1] == '\r' || data[eol-1] == '\n')) data[--eol] = '\0'; - sprintf(buf, "Proxy error: %.900s", - data+status); + buf = dupprintf("Proxy error: %s", data+status); plug_closing(p->plug, buf, PROXY_ERROR_GENERAL, 0); + sfree(buf); sfree(data); return 1; } diff --git a/psftp.c b/psftp.c index 93fa33f6..8ed4e20e 100644 --- a/psftp.c +++ b/psftp.c @@ -1448,40 +1448,43 @@ static int verbose = 0; */ void fatalbox(char *fmt, ...) { - char str[0x100]; /* Make the size big enough */ + char *str, *str2; va_list ap; va_start(ap, fmt); - strcpy(str, "Fatal:"); - vsprintf(str + strlen(str), fmt, ap); + str = dupvprintf(fmt, ap); + str2 = dupcat("Fatal: ", str, "\n", NULL); + sfree(str); va_end(ap); - strcat(str, "\n"); - fputs(str, stderr); + fputs(str2, stderr); + sfree(str2); cleanup_exit(1); } void modalfatalbox(char *fmt, ...) { - char str[0x100]; /* Make the size big enough */ + char *str, *str2; va_list ap; va_start(ap, fmt); - strcpy(str, "Fatal:"); - vsprintf(str + strlen(str), fmt, ap); + str = dupvprintf(fmt, ap); + str2 = dupcat("Fatal: ", str, "\n", NULL); + sfree(str); va_end(ap); - strcat(str, "\n"); - fputs(str, stderr); + fputs(str2, stderr); + sfree(str2); cleanup_exit(1); } void connection_fatal(void *frontend, char *fmt, ...) { - char str[0x100]; /* Make the size big enough */ + char *str, *str2; va_list ap; va_start(ap, fmt); - strcpy(str, "Fatal:"); - vsprintf(str + strlen(str), fmt, ap); + str = dupvprintf(fmt, ap); + str2 = dupcat("Fatal: ", str, "\n", NULL); + sfree(str); va_end(ap); - strcat(str, "\n"); - fputs(str, stderr); + fputs(str2, stderr); + sfree(str2); cleanup_exit(1); } diff --git a/puttygen.c b/puttygen.c index 956cdaf4..add94319 100644 --- a/puttygen.c +++ b/puttygen.c @@ -28,13 +28,14 @@ static char *cmdline_keyfile = NULL; void modalfatalbox(char *fmt, ...) { va_list ap; - char stuff[200]; + char *stuff; va_start(ap, fmt); - vsprintf(stuff, fmt, ap); + stuff = dupvprintf(fmt, ap); va_end(ap); MessageBox(NULL, stuff, "PuTTYgen Fatal Error", MB_SYSTEMMODAL | MB_ICONERROR | MB_OK); + sfree(stuff); exit(1); } @@ -364,10 +365,8 @@ static void setupbigedit1(HWND hwnd, int id, int idstatic, struct RSAKey *key) dec1 = bignum_decimal(key->exponent); dec2 = bignum_decimal(key->modulus); - buffer = smalloc(strlen(dec1) + strlen(dec2) + - strlen(key->comment) + 30); - sprintf(buffer, "%d %s %s %s", - bignum_bitcount(key->modulus), dec1, dec2, key->comment); + buffer = dupprintf("%d %s %s %s", bignum_bitcount(key->modulus), + dec1, dec2, key->comment); SetDlgItemText(hwnd, id, buffer); SetDlgItemText(hwnd, idstatic, "&Public key for pasting into authorized_keys file:"); @@ -1154,12 +1153,13 @@ static int CALLBACK MainDlgProc(HWND hwnd, UINT msg, int ret; FILE *fp = fopen(filename, "r"); if (fp) { - char buffer[FILENAME_MAX + 80]; + char *buffer; fclose(fp); - sprintf(buffer, "Overwrite existing file\n%.*s?", - FILENAME_MAX, filename); + buffer = dupprintf("Overwrite existing file\n%s?", + filename); ret = MessageBox(hwnd, buffer, "PuTTYgen Warning", MB_YESNO | MB_ICONWARNING); + sfree(buffer); if (ret != IDYES) break; } @@ -1197,12 +1197,13 @@ static int CALLBACK MainDlgProc(HWND hwnd, UINT msg, int ret; FILE *fp = fopen(filename, "r"); if (fp) { - char buffer[FILENAME_MAX + 80]; + char *buffer; fclose(fp); - sprintf(buffer, "Overwrite existing file\n%.*s?", - FILENAME_MAX, filename); + buffer = dupprintf("Overwrite existing file\n%s?", + filename); ret = MessageBox(hwnd, buffer, "PuTTYgen Warning", MB_YESNO | MB_ICONWARNING); + sfree(buffer); if (ret != IDYES) break; } diff --git a/raw.c b/raw.c index 7a07ea80..7aaafb37 100644 --- a/raw.c +++ b/raw.c @@ -90,9 +90,10 @@ static char *raw_init(void *frontend_handle, void **backend_handle, * Try to find host. */ { - char buf[200]; - sprintf(buf, "Looking up host \"%.170s\"", host); + char *buf; + buf = dupprintf("Looking up host \"%s\"", host); logevent(raw->frontend, buf); + sfree(buf); } addr = sk_namelookup(host, realhost); if ((err = sk_addr_error(addr))) @@ -105,10 +106,11 @@ static char *raw_init(void *frontend_handle, void **backend_handle, * Open socket. */ { - char buf[200], addrbuf[100]; + char *buf, addrbuf[100]; sk_getaddr(addr, addrbuf, 100); - sprintf(buf, "Connecting to %.100s port %d", addrbuf, port); + buf = dupprintf("Connecting to %s port %d", addrbuf, port); logevent(raw->frontend, buf); + sfree(buf); } raw->s = new_connection(addr, *realhost, port, 0, 1, nodelay, (Plug) raw); if ((err = sk_socket_error(raw->s))) diff --git a/rlogin.c b/rlogin.c index 942a07ec..b43804d4 100644 --- a/rlogin.c +++ b/rlogin.c @@ -121,9 +121,10 @@ static char *rlogin_init(void *frontend_handle, void **backend_handle, * Try to find host. */ { - char buf[200]; - sprintf(buf, "Looking up host \"%.170s\"", host); + char *buf; + buf = dupprintf("Looking up host \"%s\"", host); logevent(rlogin->frontend, buf); + sfree(buf); } addr = sk_namelookup(host, realhost); if ((err = sk_addr_error(addr))) @@ -136,10 +137,11 @@ static char *rlogin_init(void *frontend_handle, void **backend_handle, * Open socket. */ { - char buf[200], addrbuf[100]; + char *buf, addrbuf[100]; sk_getaddr(addr, addrbuf, 100); - sprintf(buf, "Connecting to %.100s port %d", addrbuf, port); + buf = dupprintf("Connecting to %s port %d", addrbuf, port); logevent(rlogin->frontend, buf); + sfree(buf); } rlogin->s = new_connection(addr, *realhost, port, 1, 0, nodelay, (Plug) rlogin); diff --git a/scp.c b/scp.c index 069f7ef9..dc439610 100644 --- a/scp.c +++ b/scp.c @@ -160,13 +160,15 @@ static void tell_str(FILE * stream, char *str) static void tell_user(FILE * stream, char *fmt, ...) { - char str[0x100]; /* Make the size big enough */ + char *str, *str2; va_list ap; va_start(ap, fmt); - vsprintf(str, fmt, ap); + str = dupvprintf(fmt, ap); va_end(ap); - strcat(str, "\n"); - tell_str(stream, str); + str2 = dupcat(str, "\n", NULL); + sfree(str); + tell_str(stream, str2); + sfree(str2); } static void gui_update_stats(char *name, unsigned long size, @@ -216,14 +218,15 @@ static void gui_update_stats(char *name, unsigned long size, */ void fatalbox(char *fmt, ...) { - char str[0x100]; /* Make the size big enough */ + char *str, *str2; va_list ap; va_start(ap, fmt); - strcpy(str, "Fatal: "); - vsprintf(str + strlen(str), fmt, ap); + str = dupvprintf(fmt, ap); + str2 = dupcat("Fatal: ", str, "\n", NULL); + sfree(str); va_end(ap); - strcat(str, "\n"); - tell_str(stderr, str); + tell_str(stderr, str2); + sfree(str2); errs++; if (gui_mode) { @@ -239,14 +242,15 @@ void fatalbox(char *fmt, ...) } void modalfatalbox(char *fmt, ...) { - char str[0x100]; /* Make the size big enough */ + char *str, *str2; va_list ap; va_start(ap, fmt); - strcpy(str, "Fatal: "); - vsprintf(str + strlen(str), fmt, ap); + str = dupvprintf(fmt, ap); + str2 = dupcat("Fatal: ", str, "\n", NULL); + sfree(str); va_end(ap); - strcat(str, "\n"); - tell_str(stderr, str); + tell_str(stderr, str2); + sfree(str2); errs++; if (gui_mode) { @@ -262,14 +266,15 @@ void modalfatalbox(char *fmt, ...) } void connection_fatal(void *frontend, char *fmt, ...) { - char str[0x100]; /* Make the size big enough */ + char *str, *str2; va_list ap; va_start(ap, fmt); - strcpy(str, "Fatal: "); - vsprintf(str + strlen(str), fmt, ap); + str = dupvprintf(fmt, ap); + str2 = dupcat("Fatal: ", str, "\n", NULL); + sfree(str); va_end(ap); - strcat(str, "\n"); - tell_str(stderr, str); + tell_str(stderr, str2); + sfree(str2); errs++; if (gui_mode) { @@ -427,14 +432,15 @@ static void ssh_scp_init(void) */ static void bump(char *fmt, ...) { - char str[0x100]; /* Make the size big enough */ + char *str, *str2; va_list ap; va_start(ap, fmt); - strcpy(str, "Fatal: "); - vsprintf(str + strlen(str), fmt, ap); + str = dupvprintf(fmt, ap); va_end(ap); - strcat(str, "\n"); - tell_str(stderr, str); + str2 = dupcat(str, "\n", NULL); + sfree(str); + tell_str(stderr, str2); + sfree(str2); errs++; if (back != NULL && back->socket(backhandle) != NULL) { @@ -1503,16 +1509,17 @@ int scp_finish_filerecv(void) */ static void run_err(const char *fmt, ...) { - char str[2048]; + char *str, *str2; va_list ap; va_start(ap, fmt); errs++; - strcpy(str, "scp: "); - vsprintf(str + strlen(str), fmt, ap); - strcat(str, "\n"); - scp_send_errmsg(str); - tell_user(stderr, "%s", str); + str = dupvprintf(fmt, ap); + str2 = dupcat("scp: ", str, "\n", NULL); + sfree(str); + scp_send_errmsg(str2); + tell_user(stderr, "%s", str2); va_end(ap); + sfree(str2); } /* @@ -1921,12 +1928,11 @@ static void toremote(int argc, char *argv[]) FindClose(fh); } - cmd = smalloc(strlen(targ) + 100); - sprintf(cmd, "scp%s%s%s%s -t %s", - verbose ? " -v" : "", - recursive ? " -r" : "", - preserve ? " -p" : "", - targetshouldbedirectory ? " -d" : "", targ); + cmd = dupprintf("scp%s%s%s%s -t %s", + verbose ? " -v" : "", + recursive ? " -r" : "", + preserve ? " -p" : "", + targetshouldbedirectory ? " -d" : "", targ); do_cmd(host, user, cmd); sfree(cmd); @@ -2025,12 +2031,11 @@ static void tolocal(int argc, char *argv[]) user = NULL; } - cmd = smalloc(strlen(src) + 100); - sprintf(cmd, "scp%s%s%s%s -f %s", - verbose ? " -v" : "", - recursive ? " -r" : "", - preserve ? " -p" : "", - targetshouldbedirectory ? " -d" : "", src); + cmd = dupprintf("scp%s%s%s%s -f %s", + verbose ? " -v" : "", + recursive ? " -r" : "", + preserve ? " -p" : "", + targetshouldbedirectory ? " -d" : "", src); do_cmd(host, user, cmd); sfree(cmd); diff --git a/ssh.c b/ssh.c index eae4cd53..2df71102 100644 --- a/ssh.c +++ b/ssh.c @@ -638,20 +638,29 @@ struct ssh_tag { int (*s_rdpkt) (Ssh ssh, unsigned char **data, int *datalen); }; -#define logevent(s) { logevent(ssh->frontend, s); \ - if ((flags & FLAG_STDERR) && (flags & FLAG_VERBOSE)) \ - { fprintf(stderr, "%s\n", s); fflush(stderr); } } +#define logevent(s) do { \ + logevent(ssh->frontend, s); \ + if ((flags & FLAG_STDERR) && (flags & FLAG_VERBOSE)) { \ + fprintf(stderr, "%s\n", s); \ + fflush(stderr); \ + } \ +} while (0) /* logevent, only printf-formatted. */ void logeventf(Ssh ssh, char *fmt, ...) { va_list ap; - char stuff[200]; + char *buf; va_start(ap, fmt); - vsprintf(stuff, fmt, ap); + buf = dupvprintf(fmt, ap); va_end(ap); - logevent(stuff); + logevent(buf); + if ((flags & FLAG_STDERR) && (flags & FLAG_VERBOSE)) { + fprintf(stderr, "%s\n", buf); + fflush(stderr); + } + sfree(buf); } #define bombout(msg) ( ssh->state = SSH_STATE_CLOSED, \ @@ -1049,30 +1058,29 @@ static int ssh2_rdpkt(Ssh ssh, unsigned char **data, int *datalen) case SSH2_MSG_DISCONNECT: { /* log reason code in disconnect message */ - char buf[256]; + char *buf; + int nowlen; int reason = GET_32BIT(ssh->pktin.data + 6); unsigned msglen = GET_32BIT(ssh->pktin.data + 10); - unsigned nowlen; + if (reason > 0 && reason < lenof(ssh2_disconnect_reasons)) { - sprintf(buf, "Received disconnect message (%s)", - ssh2_disconnect_reasons[reason]); + buf = dupprintf("Received disconnect message (%s)", + ssh2_disconnect_reasons[reason]); } else { - sprintf(buf, "Received disconnect message (unknown type %d)", - reason); + buf = dupprintf("Received disconnect message (unknown" + " type %d)", reason); } logevent(buf); - strcpy(buf, "Disconnection message text: "); - nowlen = strlen(buf); - if (msglen > sizeof(buf) - nowlen - 1) - msglen = sizeof(buf) - nowlen - 1; - memcpy(buf + nowlen, ssh->pktin.data + 14, msglen); - buf[nowlen + msglen] = '\0'; + sfree(buf); + buf = dupprintf("Disconnection message text: %n%.*s", + msglen, &nowlen, ssh->pktin.data + 14); logevent(buf); bombout((ssh,"Server sent disconnect message\ntype %d (%s):\n\"%s\"", reason, (reason > 0 && reason < lenof(ssh2_disconnect_reasons)) ? ssh2_disconnect_reasons[reason] : "unknown", buf+nowlen)); + sfree(buf); crReturn(0); } break; @@ -1612,11 +1620,15 @@ static int ssh2_pkt_getbool(Ssh ssh) } static void ssh2_pkt_getstring(Ssh ssh, char **p, int *length) { + int len; *p = NULL; *length = 0; if (ssh->pktin.length - ssh->pktin.savedpos < 4) return; - *length = GET_32BIT(ssh->pktin.data + ssh->pktin.savedpos); + len = GET_32BIT(ssh->pktin.data + ssh->pktin.savedpos); + if (len < 0) + return; + *length = len; ssh->pktin.savedpos += 4; if (ssh->pktin.length - ssh->pktin.savedpos < *length) return; @@ -2059,11 +2071,7 @@ static char *connect_to_host(Ssh ssh, char *host, int port, /* * Try to find host. */ - { - char buf[200]; - sprintf(buf, "Looking up host \"%.170s\"", host); - logevent(buf); - } + logeventf(ssh, "Looking up host \"%s\"", host); addr = sk_namelookup(host, realhost); if ((err = sk_addr_error(addr))) return err; @@ -2072,10 +2080,9 @@ static char *connect_to_host(Ssh ssh, char *host, int port, * Open socket. */ { - char buf[200], addrbuf[100]; + char addrbuf[100]; sk_getaddr(addr, addrbuf, 100); - sprintf(buf, "Connecting to %.100s port %d", addrbuf, port); - logevent(buf); + logeventf(ssh, "Connecting to %s port %d", addrbuf, port); } ssh->fn = &fn_table; ssh->s = new_connection(addr, *realhost, port, 0, 1, nodelay, (Plug) ssh); @@ -2396,11 +2403,7 @@ static int do_ssh1_login(Ssh ssh, unsigned char *in, int inlen, int ispkt) &ssh_3des); ssh->v1_cipher_ctx = ssh->cipher->make_context(); ssh->cipher->sesskey(ssh->v1_cipher_ctx, ssh->session_key); - { - char buf[256]; - sprintf(buf, "Initialised %.200s encryption", ssh->cipher->text_name); - logevent(buf); - } + logeventf(ssh, "Initialised %s encryption", ssh->cipher->text_name); ssh->crcda_ctx = crcda_make_context(); logevent("Installing CRC compensation attack detector"); @@ -2664,8 +2667,7 @@ static int do_ssh1_login(Ssh ssh, unsigned char *in, int inlen, int ispkt) char msgbuf[256]; if (flags & FLAG_VERBOSE) c_write_str(ssh, "Trying public key authentication.\r\n"); - sprintf(msgbuf, "Trying public key \"%.200s\"", cfg.keyfile); - logevent(msgbuf); + logeventf(ssh, "Trying public key \"%s\"", cfg.keyfile); type = key_type(cfg.keyfile); if (type != SSH_KEYTYPE_SSH1) { sprintf(msgbuf, "Key is of wrong type (%s)", @@ -3070,7 +3072,6 @@ static void ssh1_protocol(Ssh ssh, unsigned char *in, int inlen, int ispkt) int n; int sport,dport,sserv,dserv; char sports[256], dports[256], host[256]; - char buf[1024]; ssh->rportfwds = newtree234(ssh_rportcmp_ssh1); /* Add port forwardings. */ @@ -3100,10 +3101,8 @@ static void ssh1_protocol(Ssh ssh, unsigned char *in, int inlen, int ispkt) dserv = 1; dport = net_service_lookup(dports); if (!dport) { - sprintf(buf, - "Service lookup failed for destination port \"%s\"", - dports); - logevent(buf); + logeventf(ssh, "Service lookup failed for" + " destination port \"%s\"", dports); } } sport = atoi(sports); @@ -3112,43 +3111,38 @@ static void ssh1_protocol(Ssh ssh, unsigned char *in, int inlen, int ispkt) sserv = 1; sport = net_service_lookup(sports); if (!sport) { - sprintf(buf, - "Service lookup failed for source port \"%s\"", - sports); - logevent(buf); + logeventf(ssh, "Service lookup failed for source" + " port \"%s\"", sports); } } if (sport && dport) { if (type == 'L') { pfd_addforward(host, dport, sport, ssh); - sprintf(buf, "Local port %.*s%.*s%d%.*s forwarding to" - " %s:%.*s%.*s%d%.*s", - (int)(sserv ? strlen(sports) : 0), sports, - sserv, "(", sport, sserv, ")", - host, - (int)(dserv ? strlen(dports) : 0), dports, - dserv, "(", dport, dserv, ")"); - logevent(buf); + logeventf(ssh, "Local port %.*s%.*s%d%.*s forwarding to" + " %s:%.*s%.*s%d%.*s", + (int)(sserv ? strlen(sports) : 0), sports, + sserv, "(", sport, sserv, ")", + host, + (int)(dserv ? strlen(dports) : 0), dports, + dserv, "(", dport, dserv, ")"); } else { struct ssh_rportfwd *pf; pf = smalloc(sizeof(*pf)); strcpy(pf->dhost, host); pf->dport = dport; if (add234(ssh->rportfwds, pf) != pf) { - sprintf(buf, - "Duplicate remote port forwarding to %s:%d", - host, dport); - logevent(buf); + logeventf(ssh, + "Duplicate remote port forwarding to %s:%d", + host, dport); sfree(pf); } else { - sprintf(buf, "Requesting remote port %.*s%.*s%d%.*s" - " forward to %s:%.*s%.*s%d%.*s", - (int)(sserv ? strlen(sports) : 0), sports, - sserv, "(", sport, sserv, ")", - host, - (int)(dserv ? strlen(dports) : 0), dports, - dserv, "(", dport, dserv, ")"); - logevent(buf); + logeventf(ssh, "Requesting remote port %.*s%.*s%d%.*s" + " forward to %s:%.*s%.*s%d%.*s", + (int)(sserv ? strlen(sports) : 0), sports, + sserv, "(", sport, sserv, ")", + host, + (int)(dserv ? strlen(dports) : 0), dports, + dserv, "(", dport, dserv, ")"); send_packet(ssh, SSH1_CMSG_PORT_FORWARD_REQUEST, PKT_INT, sport, PKT_STR, host, @@ -3576,7 +3570,10 @@ static void ssh1_protocol(Ssh ssh, unsigned char *in, int inlen, int ispkt) */ static int in_commasep_string(char *needle, char *haystack, int haylen) { - int needlen = strlen(needle); + int needlen; + if (!needle || !haystack) /* protect against null pointers */ + return 0; + needlen = strlen(needle); while (1) { /* * Is it at the start of the string? @@ -3811,7 +3808,8 @@ static int do_ssh2_transport(Ssh ssh, unsigned char *in, int inlen, int ispkt) if (!ispkt) crWaitUntil(ispkt); - sha_string(&ssh->exhash, ssh->pktin.data + 5, ssh->pktin.length - 5); + if (ssh->pktin.length > 5) + sha_string(&ssh->exhash, ssh->pktin.data + 5, ssh->pktin.length - 5); /* * Now examine the other side's KEXINIT to see what we're up @@ -3872,7 +3870,8 @@ static int do_ssh2_transport(Ssh ssh, unsigned char *in, int inlen, int ispkt) } } if (!s->cscipher_tobe) { - bombout((ssh,"Couldn't agree a client-to-server cipher (available: %s)", str)); + bombout((ssh,"Couldn't agree a client-to-server cipher (available: %s)", + str ? str : "(null)")); crReturn(0); } @@ -3897,7 +3896,8 @@ static int do_ssh2_transport(Ssh ssh, unsigned char *in, int inlen, int ispkt) } } if (!s->sccipher_tobe) { - bombout((ssh,"Couldn't agree a server-to-client cipher (available: %s)", str)); + bombout((ssh,"Couldn't agree a server-to-client cipher (available: %s)", + str ? str : "(null)")); crReturn(0); } @@ -4118,26 +4118,16 @@ static int do_ssh2_transport(Ssh ssh, unsigned char *in, int inlen, int ispkt) ssh2_mkkey(ssh,s->K,s->exchange_hash,ssh->v2_session_id,'F',keyspace); ssh->scmac->setkey(ssh->sc_mac_ctx, keyspace); } - { - char buf[256]; - sprintf(buf, "Initialised %.200s client->server encryption", - ssh->cscipher->text_name); - logevent(buf); - sprintf(buf, "Initialised %.200s server->client encryption", - ssh->sccipher->text_name); - logevent(buf); - if (ssh->cscomp->text_name) { - sprintf(buf, "Initialised %.200s compression", - ssh->cscomp->text_name); - logevent(buf); - } - if (ssh->sccomp->text_name) { - sprintf(buf, "Initialised %.200s decompression", - ssh->sccomp->text_name); - logevent(buf); - } - } - + logeventf(ssh, "Initialised %.200s client->server encryption", + ssh->cscipher->text_name); + logeventf(ssh, "Initialised %.200s server->client encryption", + ssh->sccipher->text_name); + if (ssh->cscomp->text_name) + logeventf(ssh, "Initialised %s compression", + ssh->cscomp->text_name); + if (ssh->sccomp->text_name) + logeventf(ssh, "Initialised %s decompression", + ssh->sccomp->text_name); /* * If this is the first key exchange phase, we must pass the @@ -4351,12 +4341,13 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen, int ispkt) } s->username[strcspn(s->username, "\n\r")] = '\0'; } else { - char stuff[200]; + char *stuff; strncpy(s->username, cfg.username, sizeof(s->username)); s->username[sizeof(s->username)-1] = '\0'; if ((flags & FLAG_VERBOSE) || (flags & FLAG_INTERACTIVE)) { - sprintf(stuff, "Using username \"%s\".\r\n", s->username); + stuff = dupprintf("Using username \"%s\".\r\n", s->username); c_write_str(ssh, stuff); + sfree(stuff); } } s->got_username = TRUE; @@ -4392,13 +4383,15 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen, int ispkt) ssh2_userkey_loadpub(cfg.keyfile, NULL, &s->publickey_bloblen); } else { - char msgbuf[256]; + char *msgbuf; logeventf(ssh->frontend, "Unable to use this key file (%s)", key_type_to_str(keytype)); - sprintf(msgbuf, "Unable to use key file \"%.150s\" (%s)\r\n", - cfg.keyfile, key_type_to_str(keytype)); + msgbuf = dupprintf("Unable to use key file \"%.150s\"" + " (%s)\r\n", cfg.keyfile, + key_type_to_str(keytype)); c_write_str(ssh, msgbuf); + sfree(msgbuf); s->publickey_blob = NULL; } } else @@ -5112,7 +5105,6 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen, int ispkt) int n; int sport,dport,sserv,dserv; char sports[256], dports[256], host[256]; - char buf[1024]; ssh->rportfwds = newtree234(ssh_rportcmp_ssh2); /* Add port forwardings. */ @@ -5142,10 +5134,8 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen, int ispkt) dserv = 1; dport = net_service_lookup(dports); if (!dport) { - sprintf(buf, - "Service lookup failed for destination port \"%s\"", - dports); - logevent(buf); + logeventf(ssh, "Service lookup failed for destination" + " port \"%s\"", dports); } } sport = atoi(sports); @@ -5154,23 +5144,20 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen, int ispkt) sserv = 1; sport = net_service_lookup(sports); if (!sport) { - sprintf(buf, - "Service lookup failed for source port \"%s\"", - sports); - logevent(buf); + logeventf(ssh, "Service lookup failed for source" + " port \"%s\"", sports); } } if (sport && dport) { if (type == 'L') { pfd_addforward(host, dport, sport, ssh); - sprintf(buf, "Local port %.*s%.*s%d%.*s forwarding to" - " %s:%.*s%.*s%d%.*s", - (int)(sserv ? strlen(sports) : 0), sports, - sserv, "(", sport, sserv, ")", - host, - (int)(dserv ? strlen(dports) : 0), dports, - dserv, "(", dport, dserv, ")"); - logevent(buf); + logeventf(ssh, "Local port %.*s%.*s%d%.*s forwarding to" + " %s:%.*s%.*s%d%.*s", + (int)(sserv ? strlen(sports) : 0), sports, + sserv, "(", sport, sserv, ")", + host, + (int)(dserv ? strlen(dports) : 0), dports, + dserv, "(", dport, dserv, ")"); } else { struct ssh_rportfwd *pf; pf = smalloc(sizeof(*pf)); @@ -5178,20 +5165,17 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen, int ispkt) pf->dport = dport; pf->sport = sport; if (add234(ssh->rportfwds, pf) != pf) { - sprintf(buf, - "Duplicate remote port forwarding to %s:%d", - host, dport); - logevent(buf); + logeventf(ssh, "Duplicate remote port forwarding" + " to %s:%d", host, dport); sfree(pf); } else { - sprintf(buf, "Requesting remote port %.*s%.*s%d%.*s" - " forward to %s:%.*s%.*s%d%.*s", - (int)(sserv ? strlen(sports) : 0), sports, - sserv, "(", sport, sserv, ")", - host, - (int)(dserv ? strlen(dports) : 0), dports, - dserv, "(", dport, dserv, ")"); - logevent(buf); + logeventf(ssh, "Requesting remote port %.*s%.*s%d%.*s" + " forward to %s:%.*s%.*s%d%.*s", + (int)(sserv ? strlen(sports) : 0), sports, + sserv, "(", sport, sserv, ")", + host, + (int)(dserv ? strlen(dports) : 0), dports, + dserv, "(", dport, dserv, ")"); ssh2_pkt_init(ssh, SSH2_MSG_GLOBAL_REQUEST); ssh2_pkt_addstring(ssh, "tcpip-forward"); ssh2_pkt_addbool(ssh, 1);/* want reply */ @@ -5728,13 +5712,10 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen, int ispkt) } else { char *e = pfd_newconnect(&c->u.pfd.s, realpf->dhost, realpf->dport, c); - char buf[1024]; - sprintf(buf, "Received remote port open request for %s:%d", - realpf->dhost, realpf->dport); - logevent(buf); + logeventf(ssh, "Received remote port open request" + " for %s:%d", realpf->dhost, realpf->dport); if (e != NULL) { - sprintf(buf, "Port open failed: %s", e); - logevent(buf); + logeventf(ssh, "Port open failed: %s", e); error = "Port open failed"; } else { logevent("Forwarded port opened successfully"); @@ -6092,10 +6073,8 @@ void ssh_send_port_open(void *channel, char *hostname, int port, char *org) { struct ssh_channel *c = (struct ssh_channel *)channel; Ssh ssh = c->ssh; - char buf[1024]; - sprintf(buf, "Opening forwarded connection to %.512s:%d", hostname, port); - logevent(buf); + logeventf(ssh, "Opening forwarded connection to %s:%d", hostname, port); if (ssh->version == 1) { send_packet(ssh, SSH1_MSG_PORT_OPEN, diff --git a/telnet.c b/telnet.c index 7e612cf6..cae64d83 100644 --- a/telnet.c +++ b/telnet.c @@ -225,17 +225,18 @@ static void c_write1(Telnet telnet, int c) static void log_option(Telnet telnet, char *sender, int cmd, int option) { - char buf[50]; + char *buf; /* * The strange-looking "" below is there to avoid a * trigraph - a double question mark followed by > maps to a * closing brace character! */ - sprintf(buf, "%s:\t%s %s", sender, - (cmd == WILL ? "WILL" : cmd == WONT ? "WONT" : - cmd == DO ? "DO" : cmd == DONT ? "DONT" : ""), - telopt(option)); + buf = dupprintf("%s:\t%s %s", sender, + (cmd == WILL ? "WILL" : cmd == WONT ? "WONT" : + cmd == DO ? "DO" : cmd == DONT ? "DONT" : ""), + telopt(option)); logevent(telnet->frontend, buf); + sfree(buf); } static void send_opt(Telnet telnet, int cmd, int option) @@ -372,7 +373,7 @@ static void process_subneg(Telnet telnet) switch (telnet->sb_opt) { case TELOPT_TSPEED: if (telnet->sb_len == 1 && telnet->sb_buf[0] == TELQUAL_SEND) { - char logbuf[sizeof(cfg.termspeed) + 80]; + char *logbuf; b[0] = IAC; b[1] = SB; b[2] = TELOPT_TSPEED; @@ -383,14 +384,15 @@ static void process_subneg(Telnet telnet) b[n + 1] = SE; telnet->bufsize = sk_write(telnet->s, b, n + 2); logevent(telnet->frontend, "server:\tSB TSPEED SEND"); - sprintf(logbuf, "client:\tSB TSPEED IS %s", cfg.termspeed); + logbuf = dupprintf("client:\tSB TSPEED IS %s", cfg.termspeed); logevent(telnet->frontend, logbuf); + sfree(logbuf); } else logevent(telnet->frontend, "server:\tSB TSPEED "); break; case TELOPT_TTYPE: if (telnet->sb_len == 1 && telnet->sb_buf[0] == TELQUAL_SEND) { - char logbuf[sizeof(cfg.termtype) + 80]; + char *logbuf; b[0] = IAC; b[1] = SB; b[2] = TELOPT_TTYPE; @@ -405,8 +407,9 @@ static void process_subneg(Telnet telnet) telnet->bufsize = sk_write(telnet->s, b, n + 6); b[n + 4] = 0; logevent(telnet->frontend, "server:\tSB TTYPE SEND"); - sprintf(logbuf, "client:\tSB TTYPE IS %s", b + 4); + logbuf = dupprintf("client:\tSB TTYPE IS %s", b + 4); logevent(telnet->frontend, logbuf); + sfree(logbuf); } else logevent(telnet->frontend, "server:\tSB TTYPE \r\n"); break; @@ -415,10 +418,11 @@ static void process_subneg(Telnet telnet) p = telnet->sb_buf; q = p + telnet->sb_len; if (p < q && *p == TELQUAL_SEND) { - char logbuf[50]; + char *logbuf; p++; - sprintf(logbuf, "server:\tSB %s SEND", telopt(telnet->sb_opt)); + logbuf = dupprintf("server:\tSB %s SEND", telopt(telnet->sb_opt)); logevent(telnet->frontend, logbuf); + sfree(logbuf); if (telnet->sb_opt == TELOPT_OLD_ENVIRON) { if (cfg.rfc_environ) { value = RFC_VALUE; @@ -479,9 +483,10 @@ static void process_subneg(Telnet telnet) b[n++] = IAC; b[n++] = SE; telnet->bufsize = sk_write(telnet->s, b, n); - sprintf(logbuf, "client:\tSB %s IS %s", telopt(telnet->sb_opt), - n == 6 ? "" : ""); + logbuf = dupprintf("client:\tSB %s IS %s", telopt(telnet->sb_opt), + n == 6 ? "" : ""); logevent(telnet->frontend, logbuf); + sfree(logbuf); } break; } @@ -668,9 +673,10 @@ static char *telnet_init(void *frontend_handle, void **backend_handle, * Try to find host. */ { - char buf[200]; - sprintf(buf, "Looking up host \"%.170s\"", host); + char *buf; + buf = dupprintf("Looking up host \"%s\"", host); logevent(telnet->frontend, buf); + sfree(buf); } addr = sk_namelookup(host, realhost); if ((err = sk_addr_error(addr))) @@ -683,10 +689,11 @@ static char *telnet_init(void *frontend_handle, void **backend_handle, * Open socket. */ { - char buf[200], addrbuf[100]; + char *buf, addrbuf[100]; sk_getaddr(addr, addrbuf, 100); - sprintf(buf, "Connecting to %.100s port %d", addrbuf, port); + buf = dupprintf("Connecting to %s port %d", addrbuf, port); logevent(telnet->frontend, buf); + sfree(buf); } telnet->s = new_connection(addr, *realhost, port, 0, 1, nodelay, (Plug) telnet); @@ -772,7 +779,7 @@ static void telnet_size(void *handle, int width, int height) { Telnet telnet = (Telnet) handle; unsigned char b[16]; - char logbuf[50]; + char *logbuf; telnet->term_width = width; telnet->term_height = height; @@ -789,10 +796,11 @@ static void telnet_size(void *handle, int width, int height) b[7] = IAC; b[8] = SE; telnet->bufsize = sk_write(telnet->s, b, 9); - sprintf(logbuf, "client:\tSB NAWS %d,%d", - ((unsigned char) b[3] << 8) + (unsigned char) b[4], - ((unsigned char) b[5] << 8) + (unsigned char) b[6]); + logbuf = dupprintf("client:\tSB NAWS %d,%d", + ((unsigned char) b[3] << 8) + (unsigned char) b[4], + ((unsigned char) b[5] << 8) + (unsigned char) b[6]); logevent(telnet->frontend, logbuf); + sfree(logbuf); } /*