From 3322d4c082455f228e20a4d5553bc4b3448deb92 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 8 Dec 2018 21:03:51 +0000 Subject: [PATCH] Remove a load of obsolete printf string limits. In the previous commit I happened to notice a %.150s in a ppl_logevent call, which was probably an important safety precaution a couple of decades ago when that format string was being used for an sprintf into a fixed-size buffer, but now it's just pointless cruft. This commit removes all printf string formatting directives with a compile-time fixed size, with the one exception of a %.3s used to cut out a 3-letter month name in scpserver.c. In cases where the format string in question was already going to an arbitrary-length function like dupprintf or ppl_logevent, that's all I've done; in cases where there was still a fixed-size buffer, I've replaced it with a dynamic buffer and dupprintf. --- ssh1login.c | 5 ++-- ssh2bpp.c | 8 ++--- ssh2userauth.c | 4 +-- unix/uxpty.c | 30 ++++++++++++------- windows/windlg.c | 2 +- windows/window.c | 77 ++++++++++++++++++++++++++---------------------- 6 files changed, 70 insertions(+), 56 deletions(-) diff --git a/ssh1login.c b/ssh1login.c index dcd256e2..d3555130 100644 --- a/ssh1login.c +++ b/ssh1login.c @@ -448,8 +448,7 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl) s->keyfile = conf_get_filename(s->conf, CONF_keyfile); if (!filename_is_null(s->keyfile)) { int keytype; - ppl_logevent("Reading key file \"%.150s\"", - filename_to_str(s->keyfile)); + ppl_logevent("Reading key file \"%s\"", filename_to_str(s->keyfile)); keytype = key_type(s->keyfile); if (keytype == SSH_KEYTYPE_SSH1 || keytype == SSH_KEYTYPE_SSH1_PUBLIC) { @@ -655,7 +654,7 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl) s->cur_prompt->to_server = false; s->cur_prompt->name = dupstr("SSH key passphrase"); add_prompt(s->cur_prompt, - dupprintf("Passphrase for key \"%.100s\": ", + dupprintf("Passphrase for key \"%s\": ", s->publickey_comment), false); s->userpass_ret = seat_get_userpass_input( s->ppl.seat, s->cur_prompt, NULL); diff --git a/ssh2bpp.c b/ssh2bpp.c index d9025c3a..67d8306f 100644 --- a/ssh2bpp.c +++ b/ssh2bpp.c @@ -113,7 +113,7 @@ void ssh2_bpp_new_outgoing_crypto( (ssh2_cipher_alg(s->out.cipher)->flags & SSH_CIPHER_IS_CBC) && !(s->bpp.remote_bugs & BUG_CHOKES_ON_SSH2_IGNORE)); - bpp_logevent("Initialised %.200s outbound encryption", + bpp_logevent("Initialised %s outbound encryption", ssh2_cipher_alg(s->out.cipher)->text_name); } else { s->out.cipher = NULL; @@ -124,7 +124,7 @@ void ssh2_bpp_new_outgoing_crypto( s->out.mac = ssh2_mac_new(mac, s->out.cipher); mac->setkey(s->out.mac, mac_key); - bpp_logevent("Initialised %.200s outbound MAC algorithm%s%s", + bpp_logevent("Initialised %s outbound MAC algorithm%s%s", ssh2_mac_alg(s->out.mac)->text_name, etm_mode ? " (in ETM mode)" : "", (s->out.cipher && @@ -176,7 +176,7 @@ void ssh2_bpp_new_incoming_crypto( ssh2_cipher_setkey(s->in.cipher, ckey); ssh2_cipher_setiv(s->in.cipher, iv); - bpp_logevent("Initialised %.200s inbound encryption", + bpp_logevent("Initialised %s inbound encryption", ssh2_cipher_alg(s->in.cipher)->text_name); } else { s->in.cipher = NULL; @@ -186,7 +186,7 @@ void ssh2_bpp_new_incoming_crypto( s->in.mac = ssh2_mac_new(mac, s->in.cipher); mac->setkey(s->in.mac, mac_key); - bpp_logevent("Initialised %.200s inbound MAC algorithm%s%s", + bpp_logevent("Initialised %s inbound MAC algorithm%s%s", ssh2_mac_alg(s->in.mac)->text_name, etm_mode ? " (in ETM mode)" : "", (s->in.cipher && diff --git a/ssh2userauth.c b/ssh2userauth.c index 2a3ae9b1..545bf29f 100644 --- a/ssh2userauth.c +++ b/ssh2userauth.c @@ -225,7 +225,7 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl) */ if (!filename_is_null(s->keyfile)) { int keytype; - ppl_logevent("Reading key file \"%.150s\"", + ppl_logevent("Reading key file \"%s\"", filename_to_str(s->keyfile)); keytype = key_type(s->keyfile); if (keytype == SSH_KEYTYPE_SSH2 || @@ -798,7 +798,7 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl) s->cur_prompt->to_server = false; s->cur_prompt->name = dupstr("SSH key passphrase"); add_prompt(s->cur_prompt, - dupprintf("Passphrase for key \"%.100s\": ", + dupprintf("Passphrase for key \"%s\": ", s->publickey_comment), false); s->userpass_ret = seat_get_userpass_input( diff --git a/unix/uxpty.c b/unix/uxpty.c index 8dd82d42..97953c6e 100644 --- a/unix/uxpty.c +++ b/unix/uxpty.c @@ -707,21 +707,29 @@ static void pty_real_select_result(Pty *pty, int fd, int event, int status) close_on_exit = conf_get_int(pty->conf, CONF_close_on_exit); if (close_on_exit == FORCE_OFF || (close_on_exit == AUTO && pty->exit_code != 0)) { - char message[512]; - message[0] = '\0'; - if (WIFEXITED(pty->exit_code)) - sprintf(message, "\r\n[pterm: process terminated with exit" - " code %d]\r\n", WEXITSTATUS(pty->exit_code)); - else if (WIFSIGNALED(pty->exit_code)) + char *message; + if (WIFEXITED(pty->exit_code)) { + message = dupprintf( + "\r\n[pterm: process terminated with exit code %d]\r\n", + WEXITSTATUS(pty->exit_code)); + } else if (WIFSIGNALED(pty->exit_code)) { #ifdef HAVE_NO_STRSIGNAL - sprintf(message, "\r\n[pterm: process terminated on signal" - " %d]\r\n", WTERMSIG(pty->exit_code)); + message = dupprintf( + "\r\n[pterm: process terminated on signal %d]\r\n", + WTERMSIG(pty->exit_code)); #else - sprintf(message, "\r\n[pterm: process terminated on signal" - " %d (%.400s)]\r\n", WTERMSIG(pty->exit_code), - strsignal(WTERMSIG(pty->exit_code))); + message = dupprintf( + "\r\n[pterm: process terminated on signal %d (%s)]\r\n", + WTERMSIG(pty->exit_code), + strsignal(WTERMSIG(pty->exit_code))); #endif + } else { + /* _Shouldn't_ happen, but if it does, a vague message + * is better than no message at all */ + message = dupprintf("\r\n[pterm: process terminated]\r\n"); + } seat_stdout(pty->seat, message, strlen(message)); + sfree(message); } seat_eof(pty->seat); diff --git a/windows/windlg.c b/windows/windlg.c index 16abe3ff..e046a83e 100644 --- a/windows/windlg.c +++ b/windows/windlg.c @@ -910,7 +910,7 @@ int win_seat_confirm_weak_crypto_primitive( static const char mbtitle[] = "%s Security Alert"; static const char msg[] = "The first %s supported by the server\n" - "is %.64s, which is below the configured\n" + "is %s, which is below the configured\n" "warning threshold.\n" "Do you want to continue with this connection?\n"; char *message, *title; diff --git a/windows/window.c b/windows/window.c index f579f960..dab900cc 100644 --- a/windows/window.c +++ b/windows/window.c @@ -354,7 +354,7 @@ static void start_backend(void) { const struct BackendVtable *vt; const char *error; - char msg[1024], *title; + const char *title; char *realhost; int i; @@ -379,17 +379,19 @@ static void start_backend(void) conf_get_bool(conf, CONF_tcp_keepalives)); if (error) { char *str = dupprintf("%s Error", appname); - sprintf(msg, "Unable to open connection to\n" - "%.800s\n" "%s", conf_dest(conf), error); + char *msg = dupprintf("Unable to open connection to\n%s\n%s", + conf_dest(conf), error); MessageBox(NULL, msg, str, MB_ICONERROR | MB_OK); sfree(str); + sfree(msg); exit(0); } window_name = icon_name = NULL; + char *title_to_free = NULL; title = conf_get_str(conf, CONF_wintitle); if (!*title) { - sprintf(msg, "%s - %s", realhost, appname); - title = msg; + title_to_free = dupprintf("%s - %s", realhost, appname); + title = title_to_free; } sfree(realhost); win_set_title(wintw, title); @@ -417,17 +419,20 @@ static void start_backend(void) } session_closed = false; + + sfree(title_to_free); } static void close_session(void *ignored_context) { - char morestuff[100]; + char *newtitle; int i; session_closed = true; - sprintf(morestuff, "%.70s (inactive)", appname); - win_set_icon_title(wintw, morestuff); - win_set_title(wintw, morestuff); + newtitle = dupprintf("%s (inactive)", appname); + win_set_icon_title(wintw, newtitle); + win_set_title(wintw, newtitle); + sfree(newtitle); if (ldisc) { ldisc_free(ldisc); @@ -1178,10 +1183,9 @@ static void wintw_set_raw_mouse_mode(TermWin *tw, bool activate) */ static void win_seat_connection_fatal(Seat *seat, const char *msg) { - char title[100]; - - sprintf(title, "%.70s Fatal Error", appname); + char *title = dupprintf("%s Fatal Error", appname); MessageBox(hwnd, msg, title, MB_ICONERROR | MB_OK); + sfree(title); if (conf_get_int(conf, CONF_close_on_exit) == FORCE_ON) PostQuitMessage(1); @@ -1196,14 +1200,15 @@ static void win_seat_connection_fatal(Seat *seat, const char *msg) void cmdline_error(const char *fmt, ...) { va_list ap; - char *stuff, morestuff[100]; + char *message, *title; va_start(ap, fmt); - stuff = dupvprintf(fmt, ap); + message = dupvprintf(fmt, ap); va_end(ap); - sprintf(morestuff, "%.70s Command Line Error", appname); - MessageBox(hwnd, stuff, morestuff, MB_ICONERROR | MB_OK); - sfree(stuff); + title = dupprintf("%s Command Line Error", appname); + MessageBox(hwnd, message, title, MB_ICONERROR | MB_OK); + sfree(message); + sfree(title); exit(1); } @@ -5381,15 +5386,15 @@ static void wintw_clip_request_paste(TermWin *tw, int clipboard) void modalfatalbox(const char *fmt, ...) { va_list ap; - char *stuff, morestuff[100]; + char *message, *title; va_start(ap, fmt); - stuff = dupvprintf(fmt, ap); + message = dupvprintf(fmt, ap); va_end(ap); - sprintf(morestuff, "%.70s Fatal Error", appname); - MessageBox(hwnd, stuff, morestuff, - MB_SYSTEMMODAL | MB_ICONERROR | MB_OK); - sfree(stuff); + title = dupprintf("%s Fatal Error", appname); + MessageBox(hwnd, message, title, MB_SYSTEMMODAL | MB_ICONERROR | MB_OK); + sfree(message); + sfree(title); cleanup_exit(1); } @@ -5399,14 +5404,15 @@ void modalfatalbox(const char *fmt, ...) void nonfatal(const char *fmt, ...) { va_list ap; - char *stuff, morestuff[100]; + char *message, *title; va_start(ap, fmt); - stuff = dupvprintf(fmt, ap); + message = dupvprintf(fmt, ap); va_end(ap); - sprintf(morestuff, "%.70s Error", appname); - MessageBox(hwnd, stuff, morestuff, MB_ICONERROR | MB_OK); - sfree(stuff); + title = dupprintf("%s Error", appname); + MessageBox(hwnd, message, title, MB_ICONERROR | MB_OK); + sfree(message); + sfree(title); } static bool flash_window_ex(DWORD dwFlags, UINT uCount, DWORD dwTimeout) @@ -5514,13 +5520,14 @@ static void wintw_bell(TermWin *tw, int mode) Filename *bell_wavefile = conf_get_filename(conf, CONF_bell_wavefile); if (!p_PlaySound || !p_PlaySound(bell_wavefile->path, NULL, SND_ASYNC | SND_FILENAME)) { - char buf[sizeof(bell_wavefile->path) + 80]; - char otherbuf[100]; - sprintf(buf, "Unable to play sound file\n%s\n" - "Using default sound instead", bell_wavefile->path); - sprintf(otherbuf, "%.70s Sound Error", appname); - MessageBox(hwnd, buf, otherbuf, - MB_OK | MB_ICONEXCLAMATION); + char *buf, *otherbuf; + buf = dupprintf( + "Unable to play sound file\n%s\nUsing default sound instead", + bell_wavefile->path); + otherbuf = dupprintf("%s Sound Error", appname); + MessageBox(hwnd, buf, otherbuf, MB_OK | MB_ICONEXCLAMATION); + sfree(buf); + sfree(otherbuf); conf_set_int(conf, CONF_beep, BELL_DEFAULT); } } else if (mode == BELL_PCSPEAKER) {