Serious Bug: BN.js incorrectly computes hexsting

[ricmoo]

Ethers Version

5.6.7

Search Terms

No response

Describe the Problem

There was a serious bug identified in BN.js, which results in hex strings being incorrectly computed in some cases (analysis will be performed shortly, after I get the CI off and running).

The hexstring is used extensively in ethers v5 BigNumber library, to ensure immutability. This does not affect the v6-beta branch, as it does not use BN.js.

There is a fix published in that repository, so I just need to update the version, verify correctness, and lock the versions in the package.json files.

h/t: @alexdupre

Code Snippet

// Two numbers, which *should* be different:
const a = BigNumber.from(2).pow(336);
const b = BigNumber.from(2).pow(316);

// Should log `false`, but actually logs `true`
console.log(a.eq(b))

Contract ABI

No response

Errors

No response

Environment

No response

Environment (Other)

No response

[ricmoo]

I believe this problem should not have affected most normal uses of the BigNumber class, since the smallest number affected seems to be 2 ** 336, which is a 337-bit number, while Ethereum operations would generally only operate with numbers up to 256-bit numbers.

It is possible for arbitrary math operations to have been performed outside of this range, but that still leaves a buffer of 80 bits, and even this scenario seems unlikely.

Any additional analysis or comments from others is welcome though. :)

[alexdupre]

It's true that the smallest affected number is bigger than 2^256, and that's likely the reason it wasn't discovered before, but that's something that can be reached with simple operations on uint256 numbers that may have a wide intermediate result, e.g. in the case r = a * b / c where a, b, c and the result are uint256, but a * b overflows it. Same with a modulo operation instead of division. Surely not very common, but also not impossible (especially if exploited voluntarily).

[ricmoo]

Yes, I 100% agree that accumulation could hit that limit, especially, things like averages over large numbers of values. But it is certainly rarer than my initial panic led me to believe. :)