From 34a0460f0561aa2376659775d5e1c3307a14ccde Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Tue, 21 Jan 2020 20:16:28 +0000 Subject: [PATCH] New functions to shrink a strbuf. These are better than my previous approach of just assigning to sb->len, because firstly they check by assertion that the new length is within range, and secondly they preserve the invariant that the byte stored in the buffer just after the length runs out is \0. Switched to using the new functions everywhere a grep could turn up opportunities. (cherry picked from commit 5891142aee5cab4aedd4a6be32e80fba806f3326) --- misc.h | 3 +++ pscp.c | 8 ++++---- scpserver.c | 6 +++--- ssh1login-server.c | 2 +- ssh1login.c | 2 +- ssh2kex-server.c | 2 +- ssh2transport.c | 6 +++--- ssh2userauth.c | 2 +- sshverstring.c | 3 +-- telnet.c | 4 ++-- terminal.c | 10 +++++----- testcrypt.c | 2 +- unix/gtkdlg.c | 2 +- unix/uxnet.c | 4 ++-- unix/uxstore.c | 2 +- utils.c | 14 ++++++++++++++ windows/winpgnt.c | 2 +- 17 files changed, 45 insertions(+), 29 deletions(-) diff --git a/misc.h b/misc.h index b9fb5721..7f53d4e0 100644 --- a/misc.h +++ b/misc.h @@ -73,9 +73,12 @@ strbuf *strbuf_new_nm(void); void strbuf_free(strbuf *buf); void *strbuf_append(strbuf *buf, size_t len); +void strbuf_shrink_to(strbuf *buf, size_t new_len); +void strbuf_shrink_by(strbuf *buf, size_t amount_to_remove); char *strbuf_to_str(strbuf *buf); /* does free buf, but you must free result */ void strbuf_catf(strbuf *buf, const char *fmt, ...); void strbuf_catfv(strbuf *buf, const char *fmt, va_list ap); +static inline void strbuf_clear(strbuf *buf) { strbuf_shrink_to(buf, 0); } strbuf *strbuf_new_for_agent_query(void); void strbuf_finalise_agent_query(strbuf *buf); diff --git a/pscp.c b/pscp.c index 4724622d..ef65a591 100644 --- a/pscp.c +++ b/pscp.c @@ -1306,7 +1306,7 @@ int scp_get_sink_action(struct scp_sink_action *act) act->action = SCP_SINK_RETRY; } else { act->action = SCP_SINK_DIR; - act->buf->len = 0; + strbuf_clear(act->buf); put_asciz(act->buf, stripslashes(fname, false)); act->name = act->buf->s; act->size = 0; /* duhh, it's a directory */ @@ -1326,7 +1326,7 @@ int scp_get_sink_action(struct scp_sink_action *act) * It's a file. Return SCP_SINK_FILE. */ act->action = SCP_SINK_FILE; - act->buf->len = 0; + strbuf_clear(act->buf); put_asciz(act->buf, stripslashes(fname, false)); act->name = act->buf->s; if (attrs.flags & SSH_FILEXFER_ATTR_SIZE) { @@ -1354,7 +1354,7 @@ int scp_get_sink_action(struct scp_sink_action *act) char ch; act->settime = false; - act->buf->len = 0; + strbuf_clear(act->buf); while (!done) { if (!ssh_scp_recv(&ch, 1)) @@ -1387,7 +1387,7 @@ int scp_get_sink_action(struct scp_sink_action *act) &act->mtime, &act->atime) == 2) { act->settime = true; backend_send(backend, "", 1); - act->buf->len = 0; + strbuf_clear(act->buf); continue; /* go round again */ } bump("Protocol error: Illegal time format"); diff --git a/scpserver.c b/scpserver.c index 02e2fd07..3123dc16 100644 --- a/scpserver.c +++ b/scpserver.c @@ -1074,7 +1074,7 @@ static void scp_sink_coroutine(ScpSink *scp) * Send an ack, and read a command. */ sshfwd_write(scp->sc, "\0", 1); - scp->command->len = 0; + strbuf_clear(scp->command); while (1) { crMaybeWaitUntilV(scp->input_eof || bufchain_size(&scp->data) > 0); if (scp->input_eof) @@ -1095,7 +1095,7 @@ static void scp_sink_coroutine(ScpSink *scp) /* * Parse the command. */ - scp->command->len--; /* chomp the newline */ + strbuf_shrink_by(scp->command, 1); /* chomp the newline */ scp->command_chr = scp->command->len > 0 ? scp->command->s[0] : '\0'; if (scp->command_chr == 'T') { unsigned long dummy1, dummy2; @@ -1131,7 +1131,7 @@ static void scp_sink_coroutine(ScpSink *scp) ptrlen leafname = make_ptrlen( p, scp->command->len - (p - scp->command->s)); - scp->filename_sb->len = 0; + strbuf_clear(scp->filename_sb); put_datapl(scp->filename_sb, scp->head->destpath); if (scp->head->isdir) { if (scp->filename_sb->len > 0 && diff --git a/ssh1login-server.c b/ssh1login-server.c index e7ff93de..d4988516 100644 --- a/ssh1login-server.c +++ b/ssh1login-server.c @@ -218,7 +218,7 @@ static void ssh1_login_server_process_queue(PacketProtocolLayer *ppl) if (rsa_ssh1_decrypt_pkcs1(s->sesskey, larger, data)) { mp_free(s->sesskey); s->sesskey = mp_from_bytes_be(ptrlen_from_strbuf(data)); - data->len = 0; + strbuf_clear(data); if (rsa_ssh1_decrypt_pkcs1(s->sesskey, smaller, data) && data->len == sizeof(s->session_key)) { memcpy(s->session_key, data->u, sizeof(s->session_key)); diff --git a/ssh1login.c b/ssh1login.c index 98b7fb5b..7f431ce7 100644 --- a/ssh1login.c +++ b/ssh1login.c @@ -516,7 +516,7 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl) get_rsa_ssh1_pub(s->asrc, &s->key, RSA_SSH1_EXPONENT_FIRST); end = s->asrc->pos; - s->agent_comment->len = 0; + strbuf_clear(s->agent_comment); put_datapl(s->agent_comment, get_string(s->asrc)); if (get_err(s->asrc)) { ppl_logevent("Pageant key list packet was truncated"); diff --git a/ssh2kex-server.c b/ssh2kex-server.c index 40377153..44134ccb 100644 --- a/ssh2kex-server.c +++ b/ssh2kex-server.c @@ -57,7 +57,7 @@ void ssh2kex_coroutine(struct ssh2_transport_state *s, bool *aborted) assert(s->hkey); } - s->hostkeyblob->len = 0; + strbuf_clear(s->hostkeyblob); ssh_key_public_blob(s->hkey, BinarySink_UPCAST(s->hostkeyblob)); s->hostkeydata = ptrlen_from_strbuf(s->hostkeyblob); diff --git a/ssh2transport.c b/ssh2transport.c index 223ec5b7..982cee8c 100644 --- a/ssh2transport.c +++ b/ssh2transport.c @@ -265,7 +265,7 @@ static void ssh2_mkkey( */ keylen_padded = ((keylen + hlen - 1) / hlen) * hlen; - out->len = 0; + strbuf_clear(out); key = strbuf_append(out, keylen_padded); /* First hlen bytes. */ @@ -1083,7 +1083,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl) * Construct our KEXINIT packet, in a strbuf so we can refer to it * later. */ - s->client_kexinit->len = 0; + strbuf_clear(s->client_kexinit); put_byte(s->outgoing_kexinit, SSH2_MSG_KEXINIT); random_read(strbuf_append(s->outgoing_kexinit, 16), 16); ssh2_write_kexinit_lists( @@ -1121,7 +1121,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl) s->ppl.bpp->pls->actx, pktin->type)); return; } - s->incoming_kexinit->len = 0; + strbuf_clear(s->incoming_kexinit); put_byte(s->incoming_kexinit, SSH2_MSG_KEXINIT); put_data(s->incoming_kexinit, get_ptr(pktin), get_avail(pktin)); diff --git a/ssh2userauth.c b/ssh2userauth.c index 32dde8d0..d3bbe7a9 100644 --- a/ssh2userauth.c +++ b/ssh2userauth.c @@ -620,7 +620,7 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl) /* * Save the methods string for use in error messages. */ - s->last_methods_string->len = 0; + strbuf_clear(s->last_methods_string); put_datapl(s->last_methods_string, methods); /* diff --git a/sshverstring.c b/sshverstring.c index 4828c37b..84f23fde 100644 --- a/sshverstring.c +++ b/sshverstring.c @@ -303,8 +303,7 @@ void ssh_verstring_handle_input(BinaryPacketProtocol *bpp) while (s->vstring->len > 0 && (s->vstring->s[s->vstring->len-1] == '\r' || s->vstring->s[s->vstring->len-1] == '\n')) - s->vstring->len--; - s->vstring->s[s->vstring->len] = '\0'; + strbuf_shrink_by(s->vstring, 1); bpp_logevent("Remote version: %s", s->vstring->s); diff --git a/telnet.c b/telnet.c index 034de3a6..832a4434 100644 --- a/telnet.c +++ b/telnet.c @@ -580,7 +580,7 @@ static void do_telnet_read(Telnet *telnet, const char *buf, size_t len) break; case SEENSB: telnet->sb_opt = c; - telnet->sb_buf->len = 0; + strbuf_clear(telnet->sb_buf); telnet->state = SUBNEGOT; break; case SUBNEGOT: @@ -604,7 +604,7 @@ static void do_telnet_read(Telnet *telnet, const char *buf, size_t len) if (outbuf->len >= 4096) { c_write(telnet, outbuf->u, outbuf->len); - outbuf->len = 0; + strbuf_clear(outbuf); } } diff --git a/terminal.c b/terminal.c index db397e9c..a3885303 100644 --- a/terminal.c +++ b/terminal.c @@ -440,11 +440,11 @@ static void makerle(strbuf *b, termline *ldata, if (hdrsize == 0) { assert(prevpos == hdrpos + 1); runpos = hdrpos; - b->len = prevpos+prevlen; + strbuf_shrink_to(b, prevpos+prevlen); } else { memmove(b->u + prevpos+1, b->u + prevpos, prevlen); runpos = prevpos; - b->len = prevpos+prevlen+1; + strbuf_shrink_to(b, prevpos+prevlen+1); /* * Terminate the previous run of ordinary * literals. @@ -461,7 +461,7 @@ static void makerle(strbuf *b, termline *ldata, oldstate = state; makeliteral(b, c, &state); tmplen = b->len - tmppos; - b->len = tmppos; + strbuf_shrink_to(b, tmppos); if (tmplen != thislen || memcmp(b->u + runpos+1, b->u + tmppos, tmplen)) { state = oldstate; @@ -519,7 +519,7 @@ static void makerle(strbuf *b, termline *ldata, assert(hdrsize <= 128); b->u[hdrpos] = hdrsize - 1; } else { - b->len = hdrpos; + strbuf_shrink_to(b, hdrpos); } } static void makeliteral_chr(strbuf *b, termchar *c, unsigned long *state) @@ -3057,7 +3057,7 @@ static strbuf *term_input_data_from_unicode( int rv; rv = wc_to_mb(term->ucsdata->line_codepage, 0, widebuf, len, bufptr, len + 1, NULL, term->ucsdata); - buf->len = rv < 0 ? 0 : rv; + strbuf_shrink_to(buf, rv < 0 ? 0 : rv); } return buf; diff --git a/testcrypt.c b/testcrypt.c index 0035c0c0..ec7356f6 100644 --- a/testcrypt.c +++ b/testcrypt.c @@ -760,7 +760,7 @@ strbuf *rsa_ssh1_decrypt_pkcs1_wrapper(mp_int *input, RSAKey *key) /* Again, return "" on failure */ strbuf *sb = strbuf_new(); if (!rsa_ssh1_decrypt_pkcs1(input, key, sb)) - sb->len = 0; + strbuf_clear(sb); return sb; } #define rsa_ssh1_decrypt_pkcs1 rsa_ssh1_decrypt_pkcs1_wrapper diff --git a/unix/gtkdlg.c b/unix/gtkdlg.c index 85a9210b..27b1e55c 100644 --- a/unix/gtkdlg.c +++ b/unix/gtkdlg.c @@ -3835,7 +3835,7 @@ static void eventlog_list_handler(union control *ctrl, dlgparam *dp, /* * Construct the data to use as the selection. */ - es->seldata->len = 0; + strbuf_clear(es->seldata); for (i = 0; i < es->ninitial; i++) { if (dlg_listbox_issel(ctrl, dp, i)) strbuf_catf(es->seldata, "%s\n", es->events_initial[i]); diff --git a/unix/uxnet.c b/unix/uxnet.c index a1db9ca3..dbef8b23 100644 --- a/unix/uxnet.c +++ b/unix/uxnet.c @@ -252,7 +252,7 @@ SockAddr *sk_namelookup(const char *host, char **canonicalname, int address_fami return ret; } /* This way we are always sure the h->h_name is valid :) */ - realhost->len = 0; + strbuf_clear(realhost); strbuf_catf(realhost, "%s", h->h_name); for (n = 0; h->h_addr_list[n]; n++); ret->addresses = snewn(n, unsigned long); @@ -267,7 +267,7 @@ SockAddr *sk_namelookup(const char *host, char **canonicalname, int address_fami * success return from inet_addr. */ ret->superfamily = IP; - realhost->len = 0; + strbuf_clear(realhost); strbuf_catf(realhost, "%s", host); ret->addresses = snew(unsigned long); ret->naddresses = 1; diff --git a/unix/uxstore.c b/unix/uxstore.c index b3a5b8a7..9db713d1 100644 --- a/unix/uxstore.c +++ b/unix/uxstore.c @@ -564,7 +564,7 @@ bool enum_settings_next(settings_e *handle, strbuf *out) size_t baselen = fullpath->len; while ( (de = readdir(handle->dp)) != NULL ) { - fullpath->len = baselen; + strbuf_shrink_to(fullpath, baselen); put_datapl(fullpath, ptrlen_from_asciz(de->d_name)); if (stat(fullpath->s, &st) < 0 || !S_ISREG(st.st_mode)) diff --git a/utils.c b/utils.c index 3b0f75c5..667281d6 100644 --- a/utils.c +++ b/utils.c @@ -429,6 +429,20 @@ void *strbuf_append(strbuf *buf_o, size_t len) return toret; } +void strbuf_shrink_to(strbuf *buf, size_t new_len) +{ + assert(new_len <= buf->len); + buf->len = new_len; + buf->s[buf->len] = '\0'; +} + +void strbuf_shrink_by(strbuf *buf, size_t amount_to_remove) +{ + assert(amount_to_remove <= buf->len); + buf->len -= amount_to_remove; + buf->s[buf->len] = '\0'; +} + static void strbuf_BinarySink_write( BinarySink *bs, const void *data, size_t len) { diff --git a/windows/winpgnt.c b/windows/winpgnt.c index 0dbf32e8..4374f2fe 100644 --- a/windows/winpgnt.c +++ b/windows/winpgnt.c @@ -677,7 +677,7 @@ static void update_sessions(void) sb = strbuf_new(); while(ERROR_SUCCESS == RegEnumKey(hkey, index_key, buf, MAX_PATH)) { if(strcmp(buf, PUTTY_DEFAULT) != 0) { - sb->len = 0; + strbuf_clear(sb); unescape_registry_key(buf, sb); memset(&mii, 0, sizeof(mii));