Lines of code

Vulnerability details

Impact

Medium… a number of features within the protocol will not work if an approval reverts in the escrow or anywhere else

NB: Report mainly focuses on the usage of the SafeTransferLib’s safeApprove(), but bug is attached to the underlying call made to approve() which can also be seen in the PoolManager.sol contract

Click to see PoolManager’s affected code reference

    function handleTransfer(uint128 currency, address recipient, uint128 amount) public onlyGateway {
        address currencyAddress = currencyIdToAddress[currency];
        require(currencyAddress != address(0), "PoolManager/unknown-currency");
    //@audit instance of not  approving to zero first
        EscrowLike(escrow).approve(currencyAddress, address(this), amount);
        SafeTransferLib.safeTransferFrom(currencyAddress, address(escrow), recipient, amount);
    }

Proof of Concept

Pools are going to accept investments with stable coins making the chances of this happening higher, note that USDT is the most adopted stablecoin out there and Tether (USDT)'s approve() function will revert if the current approval is not zero, to protect against front-running changes of approvals.

Now take a look at Escrow.sol#L22-L26

    // --- Token approvals ---
    function approve(address token, address spender, uint256 value) external auth {
        SafeTransferLib.safeApprove(token, spender, value);
        emit Approve(token, spender, value);
    }

SafeTransferLib.safeApprove() just encodes the call with the normal ERC20 approve() selector, as can be seen here

    function safeApprove(address token, address to, uint256 value) internal {
        (bool success, bytes memory data) = token.call(abi.encodeWithSelector(IERC20.approve.selector, to, value));
        require(success && (data.length == 0 || abi.decode(data, (bool))), "SafeTransferLib/safe-approve-failed");
    }

Which means that whenever USDT or tokens that have the same implementation regarding it’s approval is used, then the approval is not going to work does not work when changing the allowance from an existing non-zero allowance value.

Tool used

Manual Review

Recommended Mitigation Steps

It is recommended to set the allowance to zero before increasing the allowance and use safeApprove/safeIncreaseAllowance.

Assessed type

Context

---

The text was updated successfully, but these errors were encountered:

All reactions