From c2d093e0dcd7a9bbb6f1913a6476eff799e603e4 Mon Sep 17 00:00:00 2001 From: daniellehrner Date: Mon, 2 Mar 2026 15:47:39 +0100 Subject: [PATCH 1/5] fix edge case for limb overflow Signed-off-by: daniellehrner --- .../org/hyperledger/besu/evm/UInt256.java | 20 ++++++++++ .../org/hyperledger/besu/evm/UInt256Test.java | 40 +++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java b/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java index b93388b7a3b..7d07c98934c 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java @@ -1226,6 +1226,10 @@ private UInt128 mulSubOverflow(final long v1, final long v0) { long carry = u0 - 1 + ((Long.compareUnsigned(v0, z0) <= 0) ? 1 : 0); long z1 = v1 + u1 - carry; + // q = MAX may still be 1 too high; check if result >= modulus (i.e. negative wrapped) + if (Long.compareUnsigned(z1, u1) > 0 || (z1 == u1 && Long.compareUnsigned(z0, u0) >= 0)) { + return addBack(z1, z0); + } return new UInt128(z1, z0); } @@ -1427,6 +1431,13 @@ private UInt192 mulSubOverflow(final long v2, final long v1, final long v0) { carry = u1 - 1 + ((Long.compareUnsigned(v1, res) < 0) ? 1 : 0); long z2 = v2 - carry + u2 - borrow; + // q = MAX may still be 1 too high; check if result >= modulus (i.e. negative wrapped) + if (Long.compareUnsigned(z2, u2) > 0 + || (z2 == u2 + && (Long.compareUnsigned(z1, u1) > 0 + || (z1 == u1 && Long.compareUnsigned(z0, u0) >= 0)))) { + return addBack(z2, z1, z0); + } return new UInt192(z2, z1, z0); } @@ -1660,6 +1671,15 @@ private UInt256 mulSubOverflow(final long v3, final long v2, final long v1, fina carry = u2 - 1 + ((Long.compareUnsigned(v2, res) < 0) ? 1 : 0); long z3 = v3 + u3 - carry - borrow; + // q = MAX may still be 1 too high; check if result >= modulus (i.e. negative wrapped) + if (Long.compareUnsigned(z3, u3) > 0 + || (z3 == u3 + && (Long.compareUnsigned(z2, u2) > 0 + || (z2 == u2 + && (Long.compareUnsigned(z1, u1) > 0 + || (z1 == u1 && Long.compareUnsigned(z0, u0) >= 0)))))) { + return addBack(z3, z2, z1, z0); + } return new UInt256(z3, z2, z1, z0); } diff --git a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java index fa9db2b6b23..3492dcb6f45 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java @@ -544,6 +544,46 @@ public void mulMod() { } } + @Test + public void modWithOverflowQuotientEstimate() { + // When the dividend's leading limb equals the modulus's leading limb, the trial quotient + // overflows and is clamped to 2^64-1. Verify correctness for each Modulus size. + + // Modulus192 path (b.u3==0, b.u2!=0) + UInt256 a1 = + UInt256.fromBytesBE( + new BigInteger("7effffff8000000000000000000000000000000000000000d900000000000001", 16) + .toByteArray()); + UInt256 b1 = + UInt256.fromBytesBE( + new BigInteger("7effffff800000007effffff800000008000ff0000010000", 16).toByteArray()); + BigInteger expected1 = new BigInteger("7effffff800000007dff00feffff0001d901fe0000020001", 16); + assertThat(new BigInteger(1, a1.mod(b1).toBytesBE())).isEqualTo(expected1); + assertThat(new BigInteger(1, a1.signedMod(b1).toBytesBE())).isEqualTo(expected1); + + // Modulus128 path (b.u3==0, b.u2==0, b.u1!=0) + UInt256 a2 = + UInt256.fromBytesBE(new BigInteger("ffffffffffffffff0000000000000001", 16).toByteArray()); + UInt256 b2 = + UInt256.fromBytesBE(new BigInteger("ffffffffffffffff0000000000000000", 16).toByteArray()); + BigInteger expected2 = new BigInteger("1", 16); + assertThat(new BigInteger(1, a2.mod(b2).toBytesBE())).isEqualTo(expected2); + + // Modulus256 path (b.u3!=0) + UInt256 a3 = + UInt256.fromBytesBE( + new BigInteger("7effffff800000000000000000000000000000000000000000000000000000ff", 16) + .toByteArray()); + UInt256 b3 = + UInt256.fromBytesBE( + new BigInteger("7effffff800000007effffff80000000800000000000000080000000000000ff", 16) + .toByteArray()); + BigInteger aBI3 = new BigInteger(1, a3.toBytesBE()); + BigInteger bBI3 = new BigInteger(1, b3.toBytesBE()); + BigInteger expected3 = aBI3.mod(bBI3); + assertThat(new BigInteger(1, a3.mod(b3).toBytesBE())).isEqualTo(expected3); + } + @Test public void signedMod() { final Random random = new Random(432); From 65a71c2432350470949fabc3b99c45b1106dc0da Mon Sep 17 00:00:00 2001 From: Luis Pinto Date: Tue, 3 Mar 2026 13:42:28 +0000 Subject: [PATCH 2/5] tests do not trip all fixed cases Signed-off-by: Luis Pinto --- .../org/hyperledger/besu/evm/UInt256Test.java | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java index 3492dcb6f45..eac15a7796b 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java @@ -545,7 +545,7 @@ public void mulMod() { } @Test - public void modWithOverflowQuotientEstimate() { + public void mulSubOverflowWithAddBackBug() { // When the dividend's leading limb equals the modulus's leading limb, the trial quotient // overflows and is clamped to 2^64-1. Verify correctness for each Modulus size. @@ -559,29 +559,35 @@ public void modWithOverflowQuotientEstimate() { new BigInteger("7effffff800000007effffff800000008000ff0000010000", 16).toByteArray()); BigInteger expected1 = new BigInteger("7effffff800000007dff00feffff0001d901fe0000020001", 16); assertThat(new BigInteger(1, a1.mod(b1).toBytesBE())).isEqualTo(expected1); - assertThat(new BigInteger(1, a1.signedMod(b1).toBytesBE())).isEqualTo(expected1); // Modulus128 path (b.u3==0, b.u2==0, b.u1!=0) UInt256 a2 = - UInt256.fromBytesBE(new BigInteger("ffffffffffffffff0000000000000001", 16).toByteArray()); + UInt256.fromBytesBE( + new BigInteger("7effffff800000000000000000000000d900000000000001", 16).toByteArray()); UInt256 b2 = - UInt256.fromBytesBE(new BigInteger("ffffffffffffffff0000000000000000", 16).toByteArray()); - BigInteger expected2 = new BigInteger("1", 16); + UInt256.fromBytesBE( + new BigInteger("7effffff800000007fffffffffffffff", 16).toByteArray()); + BigInteger aBI2 = new BigInteger(1, a2.toBytesBE()); + BigInteger bBI2 = new BigInteger(1, b2.toBytesBE()); + BigInteger expected2 = aBI2.mod(bBI2); assertThat(new BigInteger(1, a2.mod(b2).toBytesBE())).isEqualTo(expected2); - // Modulus256 path (b.u3!=0) + // Modulus256 path (b.u3!=0) via mulMod UInt256 a3 = UInt256.fromBytesBE( - new BigInteger("7effffff800000000000000000000000000000000000000000000000000000ff", 16) + new BigInteger("7effffff8000000000000000000000000000000000000000d900000000000001", 16) .toByteArray()); - UInt256 b3 = + UInt256 x3 = + UInt256.fromBytesBE(new BigInteger("10000000000000000", 16).toByteArray()); // 2^64 + UInt256 m3 = UInt256.fromBytesBE( - new BigInteger("7effffff800000007effffff80000000800000000000000080000000000000ff", 16) + new BigInteger("7effffff800000007effffff800000008000ff00000100007effffff80000000", 16) .toByteArray()); BigInteger aBI3 = new BigInteger(1, a3.toBytesBE()); - BigInteger bBI3 = new BigInteger(1, b3.toBytesBE()); - BigInteger expected3 = aBI3.mod(bBI3); - assertThat(new BigInteger(1, a3.mod(b3).toBytesBE())).isEqualTo(expected3); + BigInteger xBI3 = new BigInteger(1, x3.toBytesBE()); + BigInteger mBI3 = new BigInteger(1, m3.toBytesBE()); + BigInteger expected3 = aBI3.multiply(xBI3).mod(mBI3); + assertThat(new BigInteger(1, a3.mulMod(x3, m3).toBytesBE())).isEqualTo(expected3); } @Test From 5230bf6bf5b19760845e729dcf56b8c30146d07c Mon Sep 17 00:00:00 2001 From: Luis Pinto Date: Tue, 3 Mar 2026 14:36:01 +0000 Subject: [PATCH 3/5] spotless Signed-off-by: Luis Pinto --- evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java index eac15a7796b..e9d2062e36a 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java @@ -565,8 +565,7 @@ public void mulSubOverflowWithAddBackBug() { UInt256.fromBytesBE( new BigInteger("7effffff800000000000000000000000d900000000000001", 16).toByteArray()); UInt256 b2 = - UInt256.fromBytesBE( - new BigInteger("7effffff800000007fffffffffffffff", 16).toByteArray()); + UInt256.fromBytesBE(new BigInteger("7effffff800000007fffffffffffffff", 16).toByteArray()); BigInteger aBI2 = new BigInteger(1, a2.toBytesBE()); BigInteger bBI2 = new BigInteger(1, b2.toBytesBE()); BigInteger expected2 = aBI2.mod(bBI2); @@ -577,8 +576,7 @@ public void mulSubOverflowWithAddBackBug() { UInt256.fromBytesBE( new BigInteger("7effffff8000000000000000000000000000000000000000d900000000000001", 16) .toByteArray()); - UInt256 x3 = - UInt256.fromBytesBE(new BigInteger("10000000000000000", 16).toByteArray()); // 2^64 + UInt256 x3 = UInt256.fromBytesBE(new BigInteger("10000000000000000", 16).toByteArray()); // 2^64 UInt256 m3 = UInt256.fromBytesBE( new BigInteger("7effffff800000007effffff800000008000ff00000100007effffff80000000", 16) From 678fca59c05e60997788403d2d1f585098a136da Mon Sep 17 00:00:00 2001 From: Luis Pinto Date: Thu, 5 Mar 2026 13:51:58 +0000 Subject: [PATCH 4/5] Add missing reduceStep for higher limb in Modulus192 Signed-off-by: Luis Pinto --- .../org/hyperledger/besu/evm/UInt256.java | 39 ++++++++++++------- .../org/hyperledger/besu/evm/UInt256Test.java | 20 ++++++++++ 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java b/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java index 7d07c98934c..83df96c3af1 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java @@ -1450,26 +1450,39 @@ private UInt192 reduceStep( } private UInt256 reduceNormalised(final UInt256 that, final int shift, final long inv) { - UInt192 r; UInt320 v = that.shiftLeftWide(shift); - if (v.u4 != 0 || Long.compareUnsigned(v.u3, u2) >= 0) { - r = reduceStep(v.u4, v.u3, v.u2, v.u1, inv); - r = reduceStep(r.u2, r.u1, r.u0, v.u0, inv); - } else { - r = reduceStep(v.u3, v.u2, v.u1, v.u0, inv); + if (Long.compareUnsigned(v.u4, u2) < 0) { + UInt192 r; + if (v.u4 != 0 || Long.compareUnsigned(v.u3, u2) >= 0) { + r = reduceStep(v.u4, v.u3, v.u2, v.u1, inv); + r = reduceStep(r.u2, r.u1, r.u0, v.u0, inv); + } else { + r = reduceStep(v.u3, v.u2, v.u1, v.u0, inv); + } + return new UInt256(0, r.u2, r.u1, r.u0).shiftRight(shift); } - return new UInt256(0, r.u2, r.u1, r.u0).shiftRight(shift); + return reduceNormalisedSlowPath(v, shift, inv); } private UInt256 reduceNormalised(final UInt257 that, final int shift, final long inv) { - UInt192 r; UInt320 v = that.shiftLeftWide(shift); - if (v.u4 != 0 || Long.compareUnsigned(v.u3, u2) >= 0) { - r = reduceStep(v.u4, v.u3, v.u2, v.u1, inv); - r = reduceStep(r.u2, r.u1, r.u0, v.u0, inv); - } else { - r = reduceStep(v.u3, v.u2, v.u1, v.u0, inv); + if (Long.compareUnsigned(v.u4, u2) < 0) { + UInt192 r; + if (v.u4 != 0 || Long.compareUnsigned(v.u3, u2) >= 0) { + r = reduceStep(v.u4, v.u3, v.u2, v.u1, inv); + r = reduceStep(r.u2, r.u1, r.u0, v.u0, inv); + } else { + r = reduceStep(v.u3, v.u2, v.u1, v.u0, inv); + } + return new UInt256(0, r.u2, r.u1, r.u0).shiftRight(shift); } + return reduceNormalisedSlowPath(v, shift, inv); + } + + private UInt256 reduceNormalisedSlowPath(final UInt320 v, final int shift, final long inv) { + UInt192 r = reduceStep(0, v.u4, v.u3, v.u2, inv); + r = reduceStep(r.u2, r.u1, r.u0, v.u1, inv); + r = reduceStep(r.u2, r.u1, r.u0, v.u0, inv); return new UInt256(0, r.u2, r.u1, r.u0).shiftRight(shift); } diff --git a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java index e9d2062e36a..06195d6e7ec 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java @@ -544,6 +544,26 @@ public void mulMod() { } } + @Test + public void addModTestReduceNormalisedTopLimb() { + UInt256 a = + UInt256.fromBytesBE( + new BigInteger("62d900c9700000000000000000023f00bc1814ff00000000000000ca22300806", 16) + .toByteArray()); + UInt256 b = + UInt256.fromBytesBE( + new BigInteger("ffffffffffffffffb4fffff4befff4f4f4d4f4f504f4f4bef5f5100b0bf4f5f6", 16).toByteArray()); + UInt256 m = + UInt256.fromBytesBE( + new BigInteger("13464637e8bdc0e53b895d7b79348a784", 16) + .toByteArray()); + BigInteger A = new BigInteger(1, a.toBytesBE()); + BigInteger B = new BigInteger(1, b.toBytesBE()); + BigInteger M = new BigInteger(1, m.toBytesBE()); + BigInteger expected = A.add(B).mod(M); + assertThat(new BigInteger(1, a.addMod(b, m).toBytesBE())).isEqualTo(expected); + } + @Test public void mulSubOverflowWithAddBackBug() { // When the dividend's leading limb equals the modulus's leading limb, the trial quotient From 23203ee7fcfc85956ba9c20c5b0b98190e360d5e Mon Sep 17 00:00:00 2001 From: Luis Pinto Date: Thu, 5 Mar 2026 17:16:31 +0000 Subject: [PATCH 5/5] spotless Signed-off-by: Luis Pinto --- .../org/hyperledger/besu/evm/UInt256Test.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java index 06195d6e7ec..e296cf22085 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java @@ -547,16 +547,15 @@ public void mulMod() { @Test public void addModTestReduceNormalisedTopLimb() { UInt256 a = - UInt256.fromBytesBE( - new BigInteger("62d900c9700000000000000000023f00bc1814ff00000000000000ca22300806", 16) - .toByteArray()); + UInt256.fromBytesBE( + new BigInteger("62d900c9700000000000000000023f00bc1814ff00000000000000ca22300806", 16) + .toByteArray()); UInt256 b = - UInt256.fromBytesBE( - new BigInteger("ffffffffffffffffb4fffff4befff4f4f4d4f4f504f4f4bef5f5100b0bf4f5f6", 16).toByteArray()); + UInt256.fromBytesBE( + new BigInteger("ffffffffffffffffb4fffff4befff4f4f4d4f4f504f4f4bef5f5100b0bf4f5f6", 16) + .toByteArray()); UInt256 m = - UInt256.fromBytesBE( - new BigInteger("13464637e8bdc0e53b895d7b79348a784", 16) - .toByteArray()); + UInt256.fromBytesBE(new BigInteger("13464637e8bdc0e53b895d7b79348a784", 16).toByteArray()); BigInteger A = new BigInteger(1, a.toBytesBE()); BigInteger B = new BigInteger(1, b.toBytesBE()); BigInteger M = new BigInteger(1, m.toBytesBE());