uber · isaacbrodsky · Oct 9, 2022 · Oct 7, 2022 · Oct 7, 2022 · Oct 7, 2022
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,8 @@ The public API of this library consists of the functions declared in file
 [h3api.h.in](./src/h3lib/include/h3api.h.in).
 
 ## [Unreleased]
+### Fixed
+- Fixed possible signed integer overflow in `h3NeighborRotations` (#707)
 
 ## [4.0.1] - 2022-09-15
 ### Fixed

diff --git a/src/apps/testapps/testGridDisk.c b/src/apps/testapps/testGridDisk.c
@@ -320,17 +320,54 @@ SUITE(gridDisk) {
     }
 
     TEST(h3NeighborRotations_identity) {
-        // This is undefined behavior, but it's helpful for it to make sense.
+        // This is not used in gridDisk, but it's helpful for it to make sense.
         H3Index origin = 0x811d7ffffffffffL;
         int rotations = 0;
         H3Index out;
         t_assertSuccess(
             h3NeighborRotations(origin, CENTER_DIGIT, &rotations, &out));
         t_assert(out == origin, "Moving to self goes to self");
+        t_assert(rotations == 0, "Expected rotations");
+    }
+
+    TEST(h3NeighborRotations_rotationsOverflow) {
+        // Check for possible signed integer overflow of `rotations`
+        H3Index origin;
+        setH3Index(&origin, 0, 0, CENTER_DIGIT);
+        // A multiple of 6, so effectively no rotation. Very close
+        // to INT32_MAX.
+        int rotations = 2147483646;
+        H3Index out;
+        t_assertSuccess(
+            h3NeighborRotations(origin, K_AXES_DIGIT, &rotations, &out));
+        H3Index expected;
+        // Determined by looking at the base cell table
+        setH3Index(&expected, 0, 1, CENTER_DIGIT);
+        t_assert(out == expected, "Expected neighbor");
+        t_assert(rotations == 5, "Expected rotations value");
+    }
+
+    TEST(h3NeighborRotations_rotationsOverflow2) {
+        // Check for possible signed integer overflow of `rotations`
+        H3Index origin;
+        setH3Index(&origin, 0, 4, CENTER_DIGIT);
+        // This modulo 6 is 1.
+        int rotations = INT32_MAX;
+        H3Index out;
+        // This will try to move in the K direction off of origin,
+        // which will be adjusted to the IK direction.
+        t_assertSuccess(
+            h3NeighborRotations(origin, JK_AXES_DIGIT, &rotations, &out));
+        H3Index expected;
+        // Determined by looking at the base cell table
+        setH3Index(&expected, 0, 0, CENTER_DIGIT);
+        t_assert(out == expected, "Expected neighbor");
+        // 1 (original value) + 4 (newRotations for IK direction) + 1 (applied
+        // when adjusting to the IK direction) = 6, 6 modulo 6 = 0
+        t_assert(rotations == 0, "Expected rotations value");
     }
 
     TEST(h3NeighborRotations_invalid) {
-        // This is undefined behavior, but it's helpful for it to make sense.
         H3Index origin = 0x811d7ffffffffffL;
         int rotations = 0;
         H3Index out;

diff --git a/src/h3lib/lib/algos.c b/src/h3lib/lib/algos.c
@@ -355,6 +355,9 @@ H3Error h3NeighborRotations(H3Index origin, Direction dir, int *rotations,
     if (dir < CENTER_DIGIT || dir >= INVALID_DIGIT) {
         return E_FAILED;
     }
+    // Ensure that rotations is modulo'd by 6 before any possible addition,
+    // to protect against signed integer overflow.
+    *rotations = *rotations % 6;
     for (int i = 0; i < *rotations; i++) {
         dir = _rotate60ccw(dir);
     }