code-423n4 / 2021-07-pooltogether-findings

0 stars 0 forks source link

MStableYieldSource.sol: Optimise balanceOf() #21

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

hickuphh3

Vulnerability details

Impact

Since mAsset is immutable and is initialized to a valid token address, a redundant extcodesize check can be avoided, just like how UniswapV3 does it.

Recommended Mitigation Steps

function mAssetBalance() private view returns (uint256) {
    (bool success, bytes memory data) =
        mAsset.staticcall(abi.encodeWithSelector(IERC20.balanceOf.selector, address(this)));
        require(success && data.length >= 32);
        return abi.decode(data, (uint256));
}

// Replace lines mAsset.balanceOf(address(this)) with mAssetBalance()
function redeemToken(uint256 mAssetAmount)
  external
  override
  nonReentrant
  returns (uint256 mAssetsActual)
{   
    uint256 mAssetBalanceBefore = mAssetBalance();

  uint256 creditsBurned = savings.redeemUnderlying(mAssetAmount);

  imBalances[msg.sender] -= creditsBurned;
  uint256 mAssetBalanceAfter = mAssetBalance();
  mAssetsActual = mAssetBalanceAfter - mAssetBalanceBefore;

  mAsset.safeTransfer(msg.sender, mAssetsActual);

  emit Redeemed(msg.sender, mAssetAmount, mAssetsActual);
}
PierrickGT commented 3 years ago

This optimization seems overkill. How much gas would we save by implementing this kind of function? It's a trade of between gas consumption and the ease of maintenance of our code, so I prefer to keep a code that will be easier to maintain in the future than introduce some complexity that may not bring much in term of gas saving.