sherlock-audit / 2023-12-arcadia-judging

19 stars 15 forks source link

0xDazai - 0xDazai - _stake() function can revert if used with USDT duo to approve not set to zero first #125

Closed sherlock-admin2 closed 9 months ago

sherlock-admin2 commented 9 months ago

0xDazai

medium

0xDazai - _stake() function can revert if used with USDT duo to approve not set to zero first

_stake() function can revert if used with USDT duo to approve not set to zero first

Medium

Summary

StakedStargateAM.sol serves as an Asset Module for the Staked Stargate Finance pools. The _stake() function is designed to stake tokens in an external staking contract.

The process involves two steps:

  1. The ERC20 token is approved to the LP_STAKING_TIME address.
  2. The LP_STAKING_TIME address then deposits the tokens into the liquidity pool (LP) contract.

Vulnerability Detail

It is important to note that the protocol is configured to operate with the USDT token. There is a potential problem related to the ERC20 approve() function. Specifically, the Tether (USDT) implementation of approve() requires the current allowance to be set to zero before it can be changed. This is a security measure to prevent front-running attacks during approval changes.

As a result of this requirement, any attempt by a user to execute the _stake() function for a second time without first resetting their allowance to zero will cause the transaction to revert.

Impact

If _stake() function is called with USDT , next time its called again with USDT it will revert because allowance is not set to 0.

Code Snippet

https://github.com/arcadia-finance/accounts-v2/blob/9b24083cb832a41fce609a94c9146e03a77330b4/src/asset-modules/Stargate-Finance/StakedStargateAM.sol#L82-L87

    function _stake(address asset, uint256 amount) internal override {
        ERC20(asset).approve(address(LP_STAKING_TIME), amount);

        // Stake asset
        LP_STAKING_TIME.deposit(assetToPid[asset], amount);
    }

Tool used

Manual Review

Recommendation

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

Duplicate of #45

sherlock-admin2 commented 9 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid: should approve to zero first: medium(6)