From acd72d4e3e47c2d81d9f3586d227069b9c87094e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 26 Jan 2013 18:01:06 -0500 Subject: [PATCH 1/3] Correctly copy microdescs/extrinfos with internal NUL bytes Fixes bug 8037; bugfix on 0.2.0.1-alpha; reported by cypherpunks. --- changes/bug8037 | 4 ++++ src/common/util.c | 14 ++++++++++++++ src/common/util.h | 3 +++ src/or/routerparse.c | 4 ++-- 4 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 changes/bug8037 diff --git a/changes/bug8037 b/changes/bug8037 new file mode 100644 index 000000000..5f3c1a3a8 --- /dev/null +++ b/changes/bug8037 @@ -0,0 +1,4 @@ + o Minor bugfixes: + - Correctly store microdescriptors and extrainfo descriptors with + an internal NUL byte. Fixes bug 8037; bugfix on 0.2.0.1-alpha. + Bug reported by "cypherpunks". diff --git a/src/common/util.c b/src/common/util.c index 49ec75dc4..71b77e20f 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -282,6 +282,20 @@ tor_memdup_(const void *mem, size_t len DMALLOC_PARAMS) return dup; } +/** As tor_memdup(), but add an extra 0 byte at the end of the resulting + * memory. */ +void * +tor_memdup_nulterm(const void *mem, size_t len DMALLOC_PARAMS) +{ + char *dup; + tor_assert(len < SIZE_T_CEILING+1); + tor_assert(mem); + dup = tor_malloc_(len+1 DMALLOC_FN_ARGS); + memcpy(dup, mem, len); + dup[len] = '\0'; + return dup; +} + /** Helper for places that need to take a function pointer to the right * spelling of "free()". */ void diff --git a/src/common/util.h b/src/common/util.h index 59c43a444..170fb232f 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -83,6 +83,8 @@ char *tor_strndup_(const char *s, size_t n DMALLOC_PARAMS) ATTR_MALLOC ATTR_NONNULL((1)); void *tor_memdup_(const void *mem, size_t len DMALLOC_PARAMS) ATTR_MALLOC ATTR_NONNULL((1)); +void *tor_memdup_nulterm_(const void *mem, size_t len DMALLOC_PARAMS) + ATTR_MALLOC ATTR_NONNULL((1)); void tor_free_(void *mem); #ifdef USE_DMALLOC extern int dmalloc_free(const char *file, const int line, void *pnt, @@ -117,6 +119,7 @@ extern int dmalloc_free(const char *file, const int line, void *pnt, #define tor_strdup(s) tor_strdup_(s DMALLOC_ARGS) #define tor_strndup(s, n) tor_strndup_(s, n DMALLOC_ARGS) #define tor_memdup(s, n) tor_memdup_(s, n DMALLOC_ARGS) +#define tor_memdup_nulterm(s, n) tor_memdup_nulterm_(s, n DMALLOC_ARGS) void tor_log_mallinfo(int severity); diff --git a/src/or/routerparse.c b/src/or/routerparse.c index b945ea6aa..23dae382f 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -1494,7 +1494,7 @@ extrainfo_parse_entry_from_string(const char *s, const char *end, extrainfo = tor_malloc_zero(sizeof(extrainfo_t)); extrainfo->cache_info.is_extrainfo = 1; if (cache_copy) - extrainfo->cache_info.signed_descriptor_body = tor_strndup(s, end-s); + extrainfo->cache_info.signed_descriptor_body = tor_memdup_nulterm(s, end-s); extrainfo->cache_info.signed_descriptor_len = end-s; memcpy(extrainfo->cache_info.signed_descriptor_digest, digest, DIGEST_LEN); @@ -4237,7 +4237,7 @@ microdescs_parse_from_string(const char *s, const char *eos, md->bodylen = start_of_next_microdesc - cp; if (copy_body) - md->body = tor_strndup(cp, md->bodylen); + md->body = tor_memdup_nulterm(cp, md->bodylen); else md->body = (char*)cp; md->off = cp - start; From 3dc52e6636b69f10717ddc10ff55426f73ca0068 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 19 Feb 2013 17:53:38 -0500 Subject: [PATCH 2/3] Add src/or/micro-revision.i to CLEANFILES in case anybody has one Fix for 7143. --- changes/bug7143 | 4 ++++ src/or/include.am | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 changes/bug7143 diff --git a/changes/bug7143 b/changes/bug7143 new file mode 100644 index 000000000..d26135ae6 --- /dev/null +++ b/changes/bug7143 @@ -0,0 +1,4 @@ + o Minor bugfixes (build): + - Add the old src/or/micro-revision.i filename to CLEANFILES. + On the off chance that somebody has one, it will go away as soon + as they run "make clean". Fix for bug 7143; bugfix on 0.2.4.1-alpha. diff --git a/src/or/include.am b/src/or/include.am index 241015488..d2be1fb1e 100644 --- a/src/or/include.am +++ b/src/or/include.am @@ -188,6 +188,6 @@ src/or/or_sha1.i: $(src_or_tor_SOURCES) $(src_or_libtor_a_SOURCES) $(ORHEADERS) touch src/or/or_sha1.i; \ fi -CLEANFILES+= micro-revision.i +CLEANFILES+= micro-revision.i src/or/micro-revision.i FORCE: From 0cf2c01dbd9b86d396a55186e0656db33c7929d8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 15 Mar 2013 13:49:04 -0400 Subject: [PATCH 3/3] Reject most directory documents with an internal NUL. (Specifically, we reject all the ones that aren't NUL-terminated, since a NUL-terminated thing can't have a NUL in the middle.) Another fix for #8037. --- changes/bug8037 | 4 ++++ src/or/routerparse.c | 9 ++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/changes/bug8037 b/changes/bug8037 index 5f3c1a3a8..989745fc3 100644 --- a/changes/bug8037 +++ b/changes/bug8037 @@ -2,3 +2,7 @@ - Correctly store microdescriptors and extrainfo descriptors with an internal NUL byte. Fixes bug 8037; bugfix on 0.2.0.1-alpha. Bug reported by "cypherpunks". + + o Minor features: + - Reject as invalid most directory objects containing a + NUL. Belt-and-suspender fix for bug 8037. diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 23dae382f..2c345ae11 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -3902,8 +3902,15 @@ tokenize_string(memarea_t *area, tor_assert(area); s = &start; - if (!end) + if (!end) { end = start+strlen(start); + } else { + /* it's only meaningful to check for nuls if we got an end-of-string ptr */ + if (memchr(start, '\0', end-start)) { + log_warn(LD_DIR, "parse error: internal NUL character."); + return -1; + } + } for (i = 0; i < NIL_; ++i) counts[i] = 0;