[tor-commits] [tor/master] Handle edge cases in the round_*_to_next_multiple_of functions

[nickm at torproject.org]

commit 09cac24373e5a13cc527bf2f32132a9479d4ae1e
Author: teor <teor2345 at gmail.com>
Date:   Thu Dec 25 20:52:10 2014 +1100

    Handle edge cases in the round_*_to_next_multiple_of functions
    
    Consistently check for overflow in round_*_to_next_multiple_of.
    
    Check all round_*_to_next_multiple_of functions with expected values.
    Check all round_*_to_next_multiple_of functions with maximal values.
    
    Related to HS stats in #13192.
---
 changes/laplace-edge-cases |    6 +++++-
 src/common/util.c          |   12 +++++++++---
 src/test/test_util.c       |   27 +++++++++++++++++++++++++--
 3 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/changes/laplace-edge-cases b/changes/laplace-edge-cases
index f8a36e1..266b119 100644
--- a/changes/laplace-edge-cases
+++ b/changes/laplace-edge-cases
@@ -3,5 +3,9 @@
        * avoid division by zero
        * avoid taking the log of zero
        * silence clang type conversion warnings using round and trunc
-    - Add tests for laplace edge cases.
+       * consistently check for overflow in round_*_to_next_multiple_of
+    - Add tests for laplace edge cases:
+       * check add_laplace_noise with maximal values
+       * check round_*_to_next_multiple_of with additional values
+       * check round_*_to_next_multiple_of with maximal values
       Related to HS stats in #13192.
diff --git a/src/common/util.c b/src/common/util.c
index 52b3e04..3e680d2 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -491,7 +491,9 @@ round_to_power_of_2(uint64_t u64)
 unsigned
 round_to_next_multiple_of(unsigned number, unsigned divisor)
 {
-  number += divisor - 1;
+  tor_assert(divisor > 0);
+  if (UINT_MAX - divisor + 1 >= number)
+    number += divisor - 1;
   number -= number % divisor;
   return number;
 }
@@ -501,7 +503,9 @@ round_to_next_multiple_of(unsigned number, unsigned divisor)
 uint32_t
 round_uint32_to_next_multiple_of(uint32_t number, uint32_t divisor)
 {
-  number += divisor - 1;
+  tor_assert(divisor > 0);
+  if (UINT32_MAX - divisor + 1 >= number)
+    number += divisor - 1;
   number -= number % divisor;
   return number;
 }
@@ -511,7 +515,9 @@ round_uint32_to_next_multiple_of(uint32_t number, uint32_t divisor)
 uint64_t
 round_uint64_to_next_multiple_of(uint64_t number, uint64_t divisor)
 {
-  number += divisor - 1;
+  tor_assert(divisor > 0);
+  if (UINT64_MAX - divisor + 1 >= number)
+    number += divisor - 1;
   number -= number % divisor;
   return number;
 }
diff --git a/src/test/test_util.c b/src/test/test_util.c
index 940f9bd..74d7227 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -4054,8 +4054,11 @@ test_util_round_to_next_multiple_of(void *arg)
   tt_u64_op(round_uint64_to_next_multiple_of(99,7), ==, 105);
   tt_u64_op(round_uint64_to_next_multiple_of(99,9), ==, 99);
 
-  tt_i64_op(round_int64_to_next_multiple_of(0,1), ==, 0);
-  tt_i64_op(round_int64_to_next_multiple_of(0,7), ==, 0);
+  tt_assert(round_uint64_to_next_multiple_of(UINT64_MAX,2) ==
+            UINT64_MAX-UINT64_MAX%2);
+
+  tt_assert(round_int64_to_next_multiple_of(0,1) == 0);
+  tt_assert(round_int64_to_next_multiple_of(0,7) == 0);
 
   tt_i64_op(round_int64_to_next_multiple_of(99,1), ==, 99);
   tt_i64_op(round_int64_to_next_multiple_of(99,7), ==, 105);
@@ -4068,6 +4071,26 @@ test_util_round_to_next_multiple_of(void *arg)
   tt_i64_op(round_int64_to_next_multiple_of(INT64_MIN,2), ==, INT64_MIN);
   tt_i64_op(round_int64_to_next_multiple_of(INT64_MAX,2), ==,
                                             INT64_MAX-INT64_MAX%2);
+
+  tt_assert(round_uint32_to_next_multiple_of(0,1) == 0);
+  tt_assert(round_uint32_to_next_multiple_of(0,7) == 0);
+
+  tt_assert(round_uint32_to_next_multiple_of(99,1) == 99);
+  tt_assert(round_uint32_to_next_multiple_of(99,7) == 105);
+  tt_assert(round_uint32_to_next_multiple_of(99,9) == 99);
+
+  tt_assert(round_uint32_to_next_multiple_of(UINT32_MAX,2) ==
+            UINT32_MAX-UINT32_MAX%2);
+
+  tt_assert(round_to_next_multiple_of(0,1) == 0);
+  tt_assert(round_to_next_multiple_of(0,7) == 0);
+
+  tt_assert(round_to_next_multiple_of(99,1) == 99);
+  tt_assert(round_to_next_multiple_of(99,7) == 105);
+  tt_assert(round_to_next_multiple_of(99,9) == 99);
+
+  tt_assert(round_to_next_multiple_of(UINT_MAX,2) ==
+            UINT_MAX-UINT_MAX%2);
  done:
   ;
 }