Merge remote-tracking branch 'onionk/inbufoverflow1-043' into ticket33131_044

This commit is contained in:
Nick Mathewson 2020-04-24 08:15:53 -04:00
commit 4dd4dbf046
6 changed files with 37 additions and 25 deletions

3
changes/bug33131 Normal file
View File

@ -0,0 +1,3 @@
o Minor bugfixes (mainloop):
- Better guard against growing a buffer past its maximum 2GB in size.
Fixes bug 33131; bugfix on 0.3.0.4-rc.

View File

@ -3814,6 +3814,12 @@ connection_buf_read_from_socket(connection_t *conn, ssize_t *max_to_read,
at_most = connection_bucket_read_limit(conn, approx_time());
}
/* Do not allow inbuf to grow past BUF_MAX_LEN. */
const ssize_t maximum = BUF_MAX_LEN - buf_datalen(conn->inbuf);
if (at_most > maximum) {
at_most = maximum;
}
slack_in_buf = buf_slack(conn->inbuf);
again:
if ((size_t)at_most > slack_in_buf && slack_in_buf >= 1024) {

View File

@ -285,7 +285,7 @@ buf_t *
buf_new_with_data(const char *cp, size_t sz)
{
/* Validate arguments */
if (!cp || sz <= 0 || sz >= INT_MAX) {
if (!cp || sz <= 0 || sz > BUF_MAX_LEN) {
return NULL;
}
@ -530,9 +530,9 @@ buf_add(buf_t *buf, const char *string, size_t string_len)
return (int)buf->datalen;
check();
if (BUG(buf->datalen >= INT_MAX))
if (BUG(buf->datalen > BUF_MAX_LEN))
return -1;
if (BUG(buf->datalen >= INT_MAX - string_len))
if (BUG(buf->datalen > BUF_MAX_LEN - string_len))
return -1;
while (string_len) {
@ -551,7 +551,7 @@ buf_add(buf_t *buf, const char *string, size_t string_len)
}
check();
tor_assert(buf->datalen < INT_MAX);
tor_assert(buf->datalen <= BUF_MAX_LEN);
return (int)buf->datalen;
}
@ -645,7 +645,7 @@ buf_get_bytes(buf_t *buf, char *string, size_t string_len)
buf_peek(buf, string, string_len);
buf_drain(buf, string_len);
check();
tor_assert(buf->datalen < INT_MAX);
tor_assert(buf->datalen <= BUF_MAX_LEN);
return (int)buf->datalen;
}
@ -660,9 +660,9 @@ buf_move_to_buf(buf_t *buf_out, buf_t *buf_in, size_t *buf_flushlen)
char b[4096];
size_t cp, len;
if (BUG(buf_out->datalen >= INT_MAX || *buf_flushlen >= INT_MAX))
if (BUG(buf_out->datalen > BUF_MAX_LEN || *buf_flushlen > BUF_MAX_LEN))
return -1;
if (BUG(buf_out->datalen >= INT_MAX - *buf_flushlen))
if (BUG(buf_out->datalen > BUF_MAX_LEN - *buf_flushlen))
return -1;
len = *buf_flushlen;
@ -670,7 +670,7 @@ buf_move_to_buf(buf_t *buf_out, buf_t *buf_in, size_t *buf_flushlen)
len = buf_in->datalen;
cp = len; /* Remember the number of bytes we intend to copy. */
tor_assert(cp < INT_MAX);
tor_assert(cp <= BUF_MAX_LEN);
while (len) {
/* This isn't the most efficient implementation one could imagine, since
* it does two copies instead of 1, but I kinda doubt that this will be
@ -692,9 +692,9 @@ buf_move_all(buf_t *buf_out, buf_t *buf_in)
tor_assert(buf_out);
if (!buf_in)
return;
if (BUG(buf_out->datalen >= INT_MAX || buf_in->datalen >= INT_MAX))
if (BUG(buf_out->datalen > BUF_MAX_LEN || buf_in->datalen > BUF_MAX_LEN))
return;
if (BUG(buf_out->datalen >= INT_MAX - buf_in->datalen))
if (BUG(buf_out->datalen > BUF_MAX_LEN - buf_in->datalen))
return;
if (buf_out->head == NULL) {
@ -748,7 +748,7 @@ buf_find_pos_of_char(char ch, buf_pos_t *out)
char *cp = memchr(chunk->data+pos, ch, chunk->datalen - pos);
if (cp) {
out->chunk = chunk;
tor_assert(cp - chunk->data < INT_MAX);
tor_assert(cp - chunk->data <= BUF_MAX_LEN);
out->pos = (int)(cp - chunk->data);
return out->chunk_pos + out->pos;
} else {
@ -764,7 +764,7 @@ buf_find_pos_of_char(char ch, buf_pos_t *out)
static inline int
buf_pos_inc(buf_pos_t *pos)
{
tor_assert(pos->pos < INT_MAX - 1);
tor_assert(pos->pos < BUF_MAX_LEN);
++pos->pos;
if (pos->pos == (ptrdiff_t)pos->chunk->datalen) {
if (!pos->chunk->next)
@ -811,7 +811,7 @@ buf_find_string_offset(const buf_t *buf, const char *s, size_t n)
buf_pos_init(buf, &pos);
while (buf_find_pos_of_char(*s, &pos) >= 0) {
if (buf_matches_at_pos(&pos, s, n)) {
tor_assert(pos.chunk_pos + pos.pos < INT_MAX);
tor_assert(pos.chunk_pos + pos.pos <= BUF_MAX_LEN);
return (int)(pos.chunk_pos + pos.pos);
} else {
if (buf_pos_inc(&pos)<0)
@ -845,7 +845,7 @@ buf_find_offset_of_char(buf_t *buf, char ch)
{
chunk_t *chunk;
ptrdiff_t offset = 0;
tor_assert(buf->datalen < INT_MAX);
tor_assert(buf->datalen <= BUF_MAX_LEN);
for (chunk = buf->head; chunk; chunk = chunk->next) {
char *cp = memchr(chunk->data, ch, chunk->datalen);
if (cp)
@ -915,7 +915,7 @@ buf_assert_ok(buf_t *buf)
for (ch = buf->head; ch; ch = ch->next) {
total += ch->datalen;
tor_assert(ch->datalen <= ch->memlen);
tor_assert(ch->datalen < INT_MAX);
tor_assert(ch->datalen <= BUF_MAX_LEN);
tor_assert(ch->data >= &ch->mem[0]);
tor_assert(ch->data <= &ch->mem[0]+ch->memlen);
if (ch->data == &ch->mem[0]+ch->memlen) {

View File

@ -29,6 +29,9 @@ void buf_free_(buf_t *buf);
void buf_clear(buf_t *buf);
buf_t *buf_copy(const buf_t *buf);
/** Maximum bytes in a buffer, inclusive. */
#define BUF_MAX_LEN (INT_MAX - 1)
MOCK_DECL(size_t, buf_datalen, (const buf_t *buf));
size_t buf_allocation(const buf_t *buf);
size_t buf_slack(const buf_t *buf);

View File

@ -76,7 +76,7 @@ read_to_chunk(buf_t *buf, chunk_t *chunk, tor_socket_t fd, size_t at_most,
chunk->datalen += read_result;
log_debug(LD_NET,"Read %ld bytes. %d on inbuf.", (long)read_result,
(int)buf->datalen);
tor_assert(read_result < INT_MAX);
tor_assert(read_result <= BUF_MAX_LEN);
return (int)read_result;
}
}
@ -103,9 +103,9 @@ buf_read_from_fd(buf_t *buf, int fd, size_t at_most,
tor_assert(reached_eof);
tor_assert(SOCKET_OK(fd));
if (BUG(buf->datalen >= INT_MAX))
if (BUG(buf->datalen > BUF_MAX_LEN))
return -1;
if (BUG(buf->datalen >= INT_MAX - at_most))
if (BUG(buf->datalen > BUF_MAX_LEN - at_most))
return -1;
while (at_most > total_read) {
@ -127,7 +127,7 @@ buf_read_from_fd(buf_t *buf, int fd, size_t at_most,
check();
if (r < 0)
return r; /* Error */
tor_assert(total_read+r < INT_MAX);
tor_assert(total_read+r <= BUF_MAX_LEN);
total_read += r;
if ((size_t)r < readlen) { /* eof, block, or no more to read. */
break;
@ -170,7 +170,7 @@ flush_chunk(tor_socket_t fd, buf_t *buf, chunk_t *chunk, size_t sz,
} else {
*buf_flushlen -= write_result;
buf_drain(buf, write_result);
tor_assert(write_result < INT_MAX);
tor_assert(write_result <= BUF_MAX_LEN);
return (int)write_result;
}
}
@ -217,7 +217,7 @@ buf_flush_to_fd(buf_t *buf, int fd, size_t sz,
if (r == 0 || (size_t)r < flushlen0) /* can't flush any more now. */
break;
}
tor_assert(flushed < INT_MAX);
tor_assert(flushed <= BUF_MAX_LEN);
return (int)flushed;
}

View File

@ -68,9 +68,9 @@ buf_read_from_tls(buf_t *buf, tor_tls_t *tls, size_t at_most)
check_no_tls_errors();
IF_BUG_ONCE(buf->datalen >= INT_MAX)
IF_BUG_ONCE(buf->datalen > BUF_MAX_LEN)
return TOR_TLS_ERROR_MISC;
IF_BUG_ONCE(buf->datalen >= INT_MAX - at_most)
IF_BUG_ONCE(buf->datalen > BUF_MAX_LEN - at_most)
return TOR_TLS_ERROR_MISC;
while (at_most > total_read) {
@ -90,7 +90,7 @@ buf_read_from_tls(buf_t *buf, tor_tls_t *tls, size_t at_most)
r = read_to_chunk_tls(buf, chunk, tls, readlen);
if (r < 0)
return r; /* Error */
tor_assert(total_read+r < INT_MAX);
tor_assert(total_read+r <= BUF_MAX_LEN);
total_read += r;
if ((size_t)r < readlen) /* eof, block, or no more to read. */
break;
@ -177,6 +177,6 @@ buf_flush_to_tls(buf_t *buf, tor_tls_t *tls, size_t flushlen,
if (r == 0) /* Can't flush any more now. */
break;
} while (sz > 0);
tor_assert(flushed < INT_MAX);
tor_assert(flushed <= BUF_MAX_LEN);
return (int)flushed;
}