diff --git a/changes/bug20384 b/changes/bug20384
new file mode 100644
index 0000000000..591015ad94
--- /dev/null
+++ b/changes/bug20384
@@ -0,0 +1,10 @@
+ o Major features (security fixes):
+ - Prevent a class of security bugs caused by treating the contents
+ of a buffer chunk as if they were a NUL-terminated string. At
+ least one such bug seems to be present in all currently used
+ versions of Tor, and would allow an attacker to remotely crash
+ most Tor instances, especially those compiled with extra compiler
+ hardening. With this defense in place, such bugs can't crash Tor,
+ though we should still fix them as they occur. Closes ticket
+ 20384 (TROVE-2016-10-001).
+
diff --git a/changes/trove-2017-001.2 b/changes/trove-2017-001.2
new file mode 100644
index 0000000000..3ef073cf9f
--- /dev/null
+++ b/changes/trove-2017-001.2
@@ -0,0 +1,8 @@
+ o Major bugfixes (parsing):
+ - Fix an integer underflow bug when comparing malformed Tor versions.
+ This bug is harmless, except when Tor has been built with
+ --enable-expensive-hardening, which would turn it into a crash;
+ or on Tor 0.2.9.1-alpha through Tor 0.2.9.8, which were built with
+ -ftrapv by default.
+ Part of TROVE-2017-001. Fixes bug 21278; bugfix on
+ 0.0.8pre1. Found by OSS-Fuzz.
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 7804af934f..1a4dc114b6 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -4830,26 +4830,37 @@ tor_version_compare(tor_version_t *a, tor_version_t *b)
int i;
tor_assert(a);
tor_assert(b);
- if ((i = a->major - b->major))
- return i;
- else if ((i = a->minor - b->minor))
- return i;
- else if ((i = a->micro - b->micro))
- return i;
- else if ((i = a->status - b->status))
- return i;
- else if ((i = a->patchlevel - b->patchlevel))
- return i;
- else if ((i = strcmp(a->status_tag, b->status_tag)))
- return i;
- else if ((i = a->svn_revision - b->svn_revision))
- return i;
- else if ((i = a->git_tag_len - b->git_tag_len))
- return i;
- else if (a->git_tag_len)
- return fast_memcmp(a->git_tag, b->git_tag, a->git_tag_len);
+
+ /* We take this approach to comparison to ensure the same (bogus!) behavior
+ * on all inputs as we would have seen before bug #21278 was fixed. The
+ * only important difference here is that this method doesn't cause
+ * a signed integer underflow.
+ */
+#define CMP(field) do { \
+ unsigned aval = (unsigned) a->field; \
+ unsigned bval = (unsigned) b->field; \
+ int result = (int) (aval - bval); \
+ if (result < 0) \
+ return -1; \
+ else if (result > 0) \
+ return 1; \
+ } while (0)
+
+ CMP(major);
+ CMP(minor);
+ CMP(micro);
+ CMP(status);
+ CMP(patchlevel);
+ if ((i = strcmp(a->status_tag, b->status_tag)))
+ return i;
+ CMP(svn_revision);
+ CMP(git_tag_len);
+ if (a->git_tag_len)
+ return fast_memcmp(a->git_tag, b->git_tag, a->git_tag_len);
else
- return 0;
+ return 0;
+
+#undef CMP
}
/** Return true iff versions a and b belong to the same series.