code-423n4 / 2022-02-anchor-findings

0 stars 0 forks source link

[WP-M6] Using `safeApprove()` is unsafe #50

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L190

Vulnerability details

safeApprove has been deprecated in favor of safeIncreaseAllowance and safeDecreaseAllowance.

Using safeApprove is unsafe as when WormholeTokenBridge does not consume all the allowance for some reason, the future call to SafeERC20.safeApprove() will revert, because safeApprove will check and revert if the current allowance is not 0.

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L190

// Allow wormhole to spend USTw from this contract.
SafeERC20.safeApprove(IERC20(token), WORMHOLE_TOKEN_BRIDGE, amount);
// Initiate token transfer.
uint64 tokenTransferSequence = WormholeTokenBridge(
    WORMHOLE_TOKEN_BRIDGE
).transferTokens(
        token,
        amount,
        TERRA_CHAIN_ID,
        TERRA_ANCHOR_BRIDGE_ADDRESS,
        0,
        TOKEN_TRANSFER_NONCE
    );

Recommendation

Consider changing to safeIncreaseAllowance():

// Allow wormhole to spend USTw from this contract.
SafeERC20.safeIncreaseAllowance(IERC20(token), WORMHOLE_TOKEN_BRIDGE, amount);
// Initiate token transfer.
uint64 tokenTransferSequence = WormholeTokenBridge(
    WORMHOLE_TOKEN_BRIDGE
).transferTokens(
        token,
        amount,
        TERRA_CHAIN_ID,
        TERRA_ANCHOR_BRIDGE_ADDRESS,
        0,
        TOKEN_TRANSFER_NONCE
    );
GalloDaSballo commented 2 years ago

Disputed per the source code from the Bridge: https://etherscan.io/address/0xb203b2057e2f08adce8f73cc99709ffdd8edffea#code#F13#L245