search

code-423n4 / 2021-10-ambire-findings

0 stars 0 forks source link

Duplicate math operations #57

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

pauliax

Vulnerability details

Impact

First perform the addition and only then check the length to avoid this duplicate math operation: require(b.length >= index + 32, "BytesLib: length"); // Arrays are prefixed by a 256 bit length parameter index += 32; Or if you want to stay with this approach, then at least consider using the 'unchecked' keyword when this addition is performed the second time as then ready know this can't overflow. Also, in function recoverAddrImpl the same operation is performed twice: sig.length - 33

Recommended Mitigation Steps

Refactor duplicate math operations.

Ivshti commented 2 years ago

resolved in https://github.com/AmbireTech/adex-protocol-eth/commit/67a9bf713e1e21f9d6e5d19dbee1964a2db0fca4

GalloDaSballo commented 2 years ago

As per the require(sig.length >= 1, "SV_SIGLEN"); you can't underflow uint8(sig[sig.length - 1]) Using unchecked does save gas

The sponsor has applied the suggestion