From 368413a321a65234c0256c4ea80c613207cf7587 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 25 Oct 2018 09:06:13 -0400 Subject: Fix possible UB in an end-of-string check in get_next_token(). Remember, you can't check to see if there are N bytes left in a buffer by doing (buf + N < end), since the buf + N computation might take you off the end of the buffer and result in undefined behavior. Fixes 28202; bugfix on 0.2.0.3-alpha. --- src/or/routerparse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 521e237be2..063cbbcdaf 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -4964,7 +4964,7 @@ get_next_token(memarea_t *area, goto check_object; obstart = *s; /* Set obstart to start of object spec */ - if (*s+16 >= eol || memchr(*s+11,'\0',eol-*s-16) || /* no short lines, */ + if (eol - *s <= 16 || memchr(*s+11,'\0',eol-*s-16) || /* no short lines, */ strcmp_len(eol-5, "-----", 5) || /* nuls or invalid endings */ (eol-*s) > MAX_UNPARSED_OBJECT_SIZE) { /* name too long */ RET_ERR("Malformed object: bad begin line"); -- cgit v1.2.3-54-g00ecf From 0878bb961f9028a81ce465702afb891a82015228 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 25 Oct 2018 09:08:02 -0400 Subject: Fix two other cases of (buf + N > end) pattern Related to fix for 28202. --- src/or/routerparse.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 063cbbcdaf..37d2d975fc 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -5299,13 +5299,13 @@ find_start_of_next_microdesc(const char *s, const char *eos) return NULL; #define CHECK_LENGTH() STMT_BEGIN \ - if (s+32 > eos) \ + if (eos - s < 32) \ return NULL; \ STMT_END #define NEXT_LINE() STMT_BEGIN \ s = memchr(s, '\n', eos-s); \ - if (!s || s+1 >= eos) \ + if (!s || eos - s <= 1) \ return NULL; \ s++; \ STMT_END @@ -5329,7 +5329,7 @@ find_start_of_next_microdesc(const char *s, const char *eos) /* Okay, now we're pointed at the first line of the microdescriptor which is not an annotation or onion-key. The next line that _is_ an annotation or onion-key is the start of the next microdescriptor. */ - while (s+32 < eos) { + while (eos - s > 32) { if (*s == '@' || !strcmpstart(s, "onion-key")) return s; NEXT_LINE(); @@ -6359,4 +6359,3 @@ routerparse_free_all(void) { dump_desc_fifo_cleanup(); } - -- cgit v1.2.3-54-g00ecf