From c8aba9b81209f1da0d034765a0a34ecba34b809e Mon Sep 17 00:00:00 2001 From: Isaac Brodsky Date: Mon, 5 Dec 2022 13:26:58 -0800 Subject: [PATCH 1/3] Fix possible signed integer overflow in ijToIjk --- src/apps/testapps/testCellToLocalIj.c | 15 +++++++++++++++ src/h3lib/lib/coordijk.c | 6 +++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/apps/testapps/testCellToLocalIj.c b/src/apps/testapps/testCellToLocalIj.c index bf38fe6e9..5262867f6 100644 --- a/src/apps/testapps/testCellToLocalIj.c +++ b/src/apps/testapps/testCellToLocalIj.c @@ -307,4 +307,19 @@ SUITE(h3ToLocalIj) { t_assert(H3_EXPORT(localIjToCell)(origin, &ij, 0, &out) == E_FAILED, "High magnitude J and I components fail"); } + + TEST(localIjToCell_overflow_particularCases) { + H3Index origin; + setH3Index(&origin, 2, 2, CENTER_DIGIT); + CoordIJ ij = {.i = 553648127, .j = -2145378272}; + H3Index out; + t_assert(H3_EXPORT(localIjToCell)(origin, &ij, 0, &out) == E_FAILED, + "Particular high magnitude J and I components fail (1)"); + + setH3Index(&origin, 2, 2, CENTER_DIGIT); + ij.i = INT32_MAX - 10; + ij.j = -11; + t_assert(H3_EXPORT(localIjToCell)(origin, &ij, 0, &out) == E_FAILED, + "Particular high magnitude J and I components fail (2)"); + } } diff --git a/src/h3lib/lib/coordijk.c b/src/h3lib/lib/coordijk.c index 636357a87..0c6c63ac5 100644 --- a/src/h3lib/lib/coordijk.c +++ b/src/h3lib/lib/coordijk.c @@ -554,10 +554,14 @@ H3Error ijToIjk(const CoordIJ *ij, CoordIJK *ijk) { // than max. If min is positive, then max is also positive, and a // positive signed integer minus another positive signed integer will // not overflow. - if (max < INT32_MIN - min) { + if (max > INT32_MAX + min) { // max - min would overflow return E_FAILED; } + if (max < INT32_MIN - min) { + // max + min would overflow + return E_FAILED; + } if (min == INT32_MIN) { // 0 - INT32_MIN would overflow return E_FAILED; From 2bf71c7333e74410c85a0ba38468cea827eaf1cd Mon Sep 17 00:00:00 2001 From: Isaac Brodsky Date: Mon, 5 Dec 2022 13:29:07 -0800 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49e4c06d8..173dae79c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ The public API of this library consists of the functions declared in file ### Fixed - Fixed possible signed integer overflow in `h3NeighborRotations` (#707) -- Fixed possible signed integer overflow in `localIjToCell` (#706) +- Fixed possible signed integer overflow in `localIjToCell` (#706, #733) ### Changed - `assert` on defensive code blocks that are not already covered. (#720) From 1309956604e9b7c0b69659385bf65b6dcdd92a34 Mon Sep 17 00:00:00 2001 From: Isaac Brodsky Date: Mon, 5 Dec 2022 13:58:13 -0800 Subject: [PATCH 3/3] reorder --- src/h3lib/lib/coordijk.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/h3lib/lib/coordijk.c b/src/h3lib/lib/coordijk.c index 0c6c63ac5..bdb23ed10 100644 --- a/src/h3lib/lib/coordijk.c +++ b/src/h3lib/lib/coordijk.c @@ -554,10 +554,6 @@ H3Error ijToIjk(const CoordIJ *ij, CoordIJK *ijk) { // than max. If min is positive, then max is also positive, and a // positive signed integer minus another positive signed integer will // not overflow. - if (max > INT32_MAX + min) { - // max - min would overflow - return E_FAILED; - } if (max < INT32_MIN - min) { // max + min would overflow return E_FAILED; @@ -566,6 +562,10 @@ H3Error ijToIjk(const CoordIJ *ij, CoordIJK *ijk) { // 0 - INT32_MIN would overflow return E_FAILED; } + if (max > INT32_MAX + min) { + // max - min would overflow + return E_FAILED; + } } _ijkNormalize(ijk);