From eb576f0a18e326ecff841af5bb74f71fab0c3a05 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 16 Jul 2007 17:46:31 +0000 Subject: r13786@catbus: nickm | 2007-07-16 13:46:28 -0400 Backport a minimal fix for bug 455: strndup a NUL-terminated copy of each router descriptor before trying to parse it. If this slows us down a lot, we will need to reconsider, but it seems far safer than the more sophisticated stuff we are trying to do to fix 455 on trunk. svn:r10846 --- ChangeLog | 3 +++ doc/TODO.012 | 2 +- src/or/routerparse.c | 13 ++++++++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index c77e2ea96f..55708d4cae 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,6 +10,9 @@ Changes in version 0.1.2.15 - 2007-07-?? - Fix eventdns.c behavior on Solaris: It is critical to include orconfig.h _before_ sys/types.h, so that we can get the expected definition of _FILE_OFFSET_BITS. + - When the cached-routers file is an even multiple of the page size, + don't run off the end and crash. (Fixes bug 455; based on idea + from croup.) o Major bugfixes (security): - Fix a possible buffer overrun when using BSD natd support. diff --git a/doc/TODO.012 b/doc/TODO.012 index 978e5f4b4c..7a8b0b4c41 100644 --- a/doc/TODO.012 +++ b/doc/TODO.012 @@ -16,7 +16,7 @@ Backport items for 0.1.2: o r10730: Don't choose guards after any never-connected-to guard. o r10760: fix possible buffer overrun in old BSD natd code o r10790: Don't include reasons in destroy cells from the origin. - - Some fix for bug 455. + o Some fix for bug 455. o r10830 if nick thinks it's a real fix - r10835 if nick thinks it's a real fix diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 9ef6249211..ff6bff53b1 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -649,6 +649,8 @@ router_parse_list_from_string(const char **s, const char *eos, { routerinfo_t *router; const char *end, *cp, *start; + char *buf; + size_t buf_len; tor_assert(s); tor_assert(*s); @@ -690,14 +692,23 @@ router_parse_list_from_string(const char **s, const char *eos, /* cp now points to the first \n before the last non-blank line in this * descriptor */ + if (eos - cp < 25) /* not long enough to hold an "end signature" */ + break; + if (strcmpstart(cp, "\n-----END SIGNATURE-----\n")) { log_info(LD_DIR, "Ignoring truncated router descriptor."); *s = end; continue; } - router = router_parse_entry_from_string(*s, end, + /* router_parse_entry_from_string isn't necessarily safe if the string + * is non-NUL-terminated. This fix is a workaround for the stable + * series only; */ + buf_len = end-*s; + buf = tor_strndup(*s, buf_len); /* nul-terminates the copy. */ + router = router_parse_entry_from_string(buf, buf+buf_len, saved_location != SAVED_IN_CACHE); + tor_free(buf); if (!router) { log_warn(LD_DIR, "Error reading router; skipping"); -- cgit v1.2.3-54-g00ecf