From ee5471f9aab55269c8c480f1f90dfeb08803ac15 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 13 Feb 2017 15:51:55 -0500 Subject: Try to check for (and prevent) buffer size INT_MAX overflow better. Possible fix or diagnostic for 21369. --- changes/bug21369_check | 3 +++ src/or/buffers.c | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 changes/bug21369_check diff --git a/changes/bug21369_check b/changes/bug21369_check new file mode 100644 index 0000000000..2cd808c9b6 --- /dev/null +++ b/changes/bug21369_check @@ -0,0 +1,3 @@ + o Minor features (reliability, crash): + - Try better to detect problems in buffers where they might grow (or + think they have grown) over 2 GB in size. Diagnostic for bug 21369. diff --git a/src/or/buffers.c b/src/or/buffers.c index 8981fd283b..fc9e7e40cd 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -562,6 +562,11 @@ read_to_buf(tor_socket_t s, size_t at_most, buf_t *buf, int *reached_eof, tor_assert(reached_eof); tor_assert(SOCKET_OK(s)); + if (BUG(buf->datalen >= INT_MAX)) + return -1; + if (BUG(buf->datalen >= INT_MAX - at_most)) + return -1; + while (at_most > total_read) { size_t readlen = at_most - total_read; chunk_t *chunk; @@ -619,6 +624,11 @@ read_to_buf_tls(tor_tls_t *tls, size_t at_most, buf_t *buf) check(); + if (BUG(buf->datalen >= INT_MAX)) + return -1; + if (BUG(buf->datalen >= INT_MAX - at_most)) + return -1; + while (at_most > total_read) { size_t readlen = at_most - total_read; chunk_t *chunk; @@ -813,6 +823,11 @@ write_to_buf(const char *string, size_t string_len, buf_t *buf) return (int)buf->datalen; check(); + if (BUG(buf->datalen >= INT_MAX)) + return -1; + if (BUG(buf->datalen >= INT_MAX - string_len)) + return -1; + while (string_len) { size_t copy; if (!buf->tail || !CHUNK_REMAINING_CAPACITY(buf->tail)) @@ -962,6 +977,12 @@ move_buf_to_buf(buf_t *buf_out, buf_t *buf_in, size_t *buf_flushlen) /* We can do way better here, but this doesn't turn up in any profiles. */ char b[4096]; size_t cp, len; + + if (BUG(buf_out->datalen >= INT_MAX)) + return -1; + if (BUG(buf_out->datalen >= INT_MAX - *buf_flushlen)) + return -1; + len = *buf_flushlen; if (len > buf_in->datalen) len = buf_in->datalen; -- cgit v1.2.3-54-g00ecf From 074f24846321b8d08a8f67c37d72018842274c4e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 27 Feb 2017 09:12:51 -0500 Subject: Add one other BUG check to try to fix/solve 21369. Teor thinks that this connection_dirserv_add_dir_bytes_to_outbuf() might be the problem, if the "remaining" calculation underflows. So I'm adding a couple of checks there, and improving the casts. --- src/or/dirserv.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index fa3938b5ec..fd9d0c768b 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -3629,8 +3629,14 @@ connection_dirserv_add_dir_bytes_to_outbuf(dir_connection_t *conn) if (bytes < 8192) bytes = 8192; remaining = conn->cached_dir->dir_z_len - conn->cached_dir_offset; - if (bytes > remaining) + if (BUG(remaining < 0)) { + remaining = 0; + } + if (bytes > remaining) { bytes = (ssize_t) remaining; + if (BUG(bytes < 0)) + return -1; + } if (conn->zlib_state) { connection_write_to_buf_zlib( @@ -3641,7 +3647,7 @@ connection_dirserv_add_dir_bytes_to_outbuf(dir_connection_t *conn) bytes, TO_CONN(conn)); } conn->cached_dir_offset += bytes; - if (conn->cached_dir_offset == (int)conn->cached_dir->dir_z_len) { + if (conn->cached_dir_offset >= (off_t)conn->cached_dir->dir_z_len) { /* We just wrote the last one; finish up. */ connection_dirserv_finish_spooling(conn); cached_dir_decref(conn->cached_dir); -- cgit v1.2.3-54-g00ecf