search

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

0 stars 0 forks source link

Consider removing `Math.sol` #35

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

Most of the functions in Math.sol are unused but one: subOrZero(uint256 a, uint256 b).

Even for subOrZero(), it's only been used 2 times, both are unnecessary as they will revert when underflows anyway.

Removing Math.sol will make the code simpler and improve readability.

https://github.com/code-423n4/2021-10-tally/blob/c585c214edb58486e0564cb53d87e4831959c08b/contracts/swap/Swap.sol#L215-L222

if(!signifiesETHOrZero(address(zrxBuyTokenAddress))) {
    erc20Delta = zrxBuyTokenAddress.balanceOf(address(this)).subOrZero(originalERC20Balance);
    require(erc20Delta > 0, "Swap::fillZrxQuote: Didn't receive bought token");
}

Recommendation

Change to:

if(!signifiesETHOrZero(address(zrxBuyTokenAddress))) {
    erc20Delta = zrxBuyTokenAddress.balanceOf(address(this)) - originalERC20Balance;
    require(erc20Delta > 0, "Swap::fillZrxQuote: Didn't receive bought token");
}