From 0a588821cb5540e901a3d5b07ac73a20905a2c64 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 30 Jul 2020 14:19:32 -0400
Subject: Add unit test for buf_move_all(), including a failing case

The failing case is #if'd out for now, but will be fixed in the next
commit.

Testing for a fix for #40076.
---
 src/test/test_buffers.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

(limited to 'src')

diff --git a/src/test/test_buffers.c b/src/test/test_buffers.c
index 2f9ad1fbe3..67a49a5017 100644
--- a/src/test/test_buffers.c
+++ b/src/test/test_buffers.c
@@ -302,6 +302,71 @@ test_buffer_pullup(void *arg)
   tor_free(tmp);
 }
 
+static void
+test_buffers_move_all(void *arg)
+{
+  (void)arg;
+  buf_t *input = buf_new();
+  buf_t *output = buf_new();
+  char *s = NULL;
+
+#if 0
+  /* Move from empty buffer to nonempty buffer. (This is a regression test for
+   * #40076) */
+  buf_add(output, "abc", 3);
+  buf_assert_ok(input);
+  buf_assert_ok(output);
+  buf_move_all(output, input);
+  buf_assert_ok(input);
+  buf_assert_ok(output);
+  tt_int_op(buf_datalen(output), OP_EQ, 3);
+  s = buf_extract(output, NULL);
+  tt_str_op(s, OP_EQ, "abc");
+  buf_free(output);
+  buf_free(input);
+  tor_free(s);
+
+  /* Move from empty to empty. */
+  output = buf_new();
+  input = buf_new();
+#endif
+  buf_move_all(output, input);
+  buf_assert_ok(input);
+  buf_assert_ok(output);
+  tt_int_op(buf_datalen(output), OP_EQ, 0);
+  buf_free(output);
+  buf_free(input);
+
+  /* Move from nonempty to empty. */
+  output = buf_new();
+  input = buf_new();
+  buf_add(input, "longstanding bugs", 17);
+  buf_move_all(output, input);
+  buf_assert_ok(input);
+  buf_assert_ok(output);
+  s = buf_extract(output, NULL);
+  tt_str_op(s, OP_EQ, "longstanding bugs");
+  buf_free(output);
+  buf_free(input);
+  tor_free(s);
+
+  /* Move from nonempty to nonempty. */
+  output = buf_new();
+  input = buf_new();
+  buf_add(output, "the start of", 12);
+  buf_add(input, " a string", 9);
+  buf_move_all(output, input);
+  buf_assert_ok(input);
+  buf_assert_ok(output);
+  s = buf_extract(output, NULL);
+  tt_str_op(s, OP_EQ, "the start of a string");
+
+ done:
+  buf_free(output);
+  buf_free(input);
+  tor_free(s);
+}
+
 static void
 test_buffer_copy(void *arg)
 {
@@ -799,6 +864,7 @@ struct testcase_t buffer_tests[] = {
   { "basic", test_buffers_basic, TT_FORK, NULL, NULL },
   { "copy", test_buffer_copy, TT_FORK, NULL, NULL },
   { "pullup", test_buffer_pullup, TT_FORK, NULL, NULL },
+  { "move_all", test_buffers_move_all, 0, NULL, NULL },
   { "startswith", test_buffer_peek_startswith, 0, NULL, NULL },
   { "allocation_tracking", test_buffer_allocation_tracking, TT_FORK,
     NULL, NULL },
-- 
cgit v1.2.3-54-g00ecf


From c4742b89b23d58958ee0d5ca324dac5948c94bf6 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 30 Jul 2020 14:24:25 -0400
Subject: Fix a bug in buf_move_all() when the input buffer is empty.

We found this in #40076, after we started using buf_move_all() in
more places.  Fixes bug #40076; bugfix on 0.3.3.1-alpha.  As far as
I know, the crash only affects master, but I think this warrants a
backport, "just in case".
---
 changes/bug40076            | 5 +++++
 src/lib/container/buffers.c | 2 ++
 src/test/test_buffers.c     | 2 --
 3 files changed, 7 insertions(+), 2 deletions(-)
 create mode 100644 changes/bug40076

(limited to 'src')

diff --git a/changes/bug40076 b/changes/bug40076
new file mode 100644
index 0000000000..9ef5969ae8
--- /dev/null
+++ b/changes/bug40076
@@ -0,0 +1,5 @@
+  o Minor bugfixes (correctness, buffers):
+    - Fix a correctness bug that could cause an assertion failure if we ever
+      tried using the buf_move_all() function with an empty input.
+      As far as we know, no released versions of Tor do this.
+      Fixes bug 40076; bugfix on 0.3.3.1-alpha.
diff --git a/src/lib/container/buffers.c b/src/lib/container/buffers.c
index 67887f2f30..fe4cf7c385 100644
--- a/src/lib/container/buffers.c
+++ b/src/lib/container/buffers.c
@@ -689,6 +689,8 @@ buf_move_all(buf_t *buf_out, buf_t *buf_in)
   tor_assert(buf_out);
   if (!buf_in)
     return;
+  if (buf_datalen(buf_in) == 0)
+    return;
   if (BUG(buf_out->datalen >= INT_MAX || buf_in->datalen >= INT_MAX))
     return;
   if (BUG(buf_out->datalen >= INT_MAX - buf_in->datalen))
diff --git a/src/test/test_buffers.c b/src/test/test_buffers.c
index 67a49a5017..3e7364a5c8 100644
--- a/src/test/test_buffers.c
+++ b/src/test/test_buffers.c
@@ -310,7 +310,6 @@ test_buffers_move_all(void *arg)
   buf_t *output = buf_new();
   char *s = NULL;
 
-#if 0
   /* Move from empty buffer to nonempty buffer. (This is a regression test for
    * #40076) */
   buf_add(output, "abc", 3);
@@ -329,7 +328,6 @@ test_buffers_move_all(void *arg)
   /* Move from empty to empty. */
   output = buf_new();
   input = buf_new();
-#endif
   buf_move_all(output, input);
   buf_assert_ok(input);
   buf_assert_ok(output);
-- 
cgit v1.2.3-54-g00ecf