# 31523 - \[SC - Low] USDT Approval will cause function failure

Submitted on May 21st 2024 at 01:03:13 UTC by @SAAJ for [Boost | Alchemix](https://immunefi.com/bounty/alchemix-boost/)

Report ID: #31523

Report type: Smart Contract

Report severity: Low

Target: <https://github.com/alchemix-finance/alchemix-v2-dao/blob/main/src/RewardPoolManager.sol>

Impacts:

* Smart contract unable to operate due to lack of token funds
* Contract fails to deliver promised returns, but doesn't lose value

## Description

## Brief/Intro

USDT approve requires resetting before changing

## Vulnerability Details

There are 2 instances where the `IERC20.approve()` function is called only once without setting the allowance to zero. USDT require first reducing the address' allowance to zero by calling `approve(_spender, 0)` before.

```
File: RewardPoolManager.sol
function depositIntoRewardPool(uint256 _amount) external returns (bool) {
        require(msg.sender == veALCX, "must be veALCX");

        IERC20(poolToken).approve(rewardPool, _amount);
        IRewardPool4626(rewardPool).deposit(_amount, address(this));
        return true;
    }
```

Same issue exist in `claim` of `RevenueHandler` contract

```
File: RevenueHandler.sol
    /// @inheritdoc IRevenueHandler
    function claim(
        uint256 tokenId,
        address token,
        address alchemist,
        uint256 amount,
        address recipient
    ) external override {
        require(IVotingEscrow(veALCX).isApprovedOrOwner(msg.sender, tokenId), "Not approved or owner");

        uint256 amountBurned = 0;

        uint256 amountClaimable = _claimable(tokenId, token);
        require(amount <= amountClaimable, "Not enough claimable");
        require(amount > 0, "Amount must be greater than 0");
        require(amount <= IERC20(token).balanceOf(address(this)), "Not enough revenue to claim");

        userCheckpoints[tokenId][token].lastClaimEpoch = currentEpoch;
        userCheckpoints[tokenId][token].unclaimed = amountClaimable - amount;

        // If the alchemist is defined we know it has an alchemic-token
        if (alchemists[alchemist] != address(0)) {
            require(token == IAlchemistV2(alchemist).debtToken(), "Invalid alchemist/alchemic-token pair");

            (, address[] memory deposits) = IAlchemistV2(alchemist).accounts(recipient);
            IERC20(token).approve(alchemist, amount);

            // Only burn if there are deposits
            amountBurned = deposits.length > 0 ? IAlchemistV2(alchemist).burn(amount, recipient) : 0;
        }
```

The usage of `approve()` in `claim` function of `RevenueHandler` at L#210 and in `depositIntoRewardPool` function of `RewardPoolManager` at L#87 does not implement the recommended approach for approving USDT tokens as provided by the USDT firm contract.

## Impact Details

The `claim` and `depositIntoRewardPool` function of contract `RevenueHandler` & `RewardPoolManager` respectively are impacted as they will revert when call are made to these function.

Reverting of above mentioned functions, making it impossible to deposit `USDT` and similar tokens into the contract for reward purpose.

## Recommended Mitigation Steps

Use `approve(_spender, 0)` to set the allowance to zero before the line each of the existing approve() calls made. Modify the `claim` function of contract `RevenueHandler` as below:

```diff
    /// @inheritdoc IRevenueHandler
    function claim(
        uint256 tokenId,
        address token,
        address alchemist,
        uint256 amount,
        address recipient
    ) external override {
        require(IVotingEscrow(veALCX).isApprovedOrOwner(msg.sender, tokenId), "Not approved or owner");

        uint256 amountBurned = 0;

        uint256 amountClaimable = _claimable(tokenId, token);
        require(amount <= amountClaimable, "Not enough claimable");
        require(amount > 0, "Amount must be greater than 0");
        require(amount <= IERC20(token).balanceOf(address(this)), "Not enough revenue to claim");

        userCheckpoints[tokenId][token].lastClaimEpoch = currentEpoch;
        userCheckpoints[tokenId][token].unclaimed = amountClaimable - amount;

        // If the alchemist is defined we know it has an alchemic-token
        if (alchemists[alchemist] != address(0)) {
            require(token == IAlchemistV2(alchemist).debtToken(), "Invalid alchemist/alchemic-token pair");

            (, address[] memory deposits) = IAlchemistV2(alchemist).accounts(recipient);

+	         IERC20(token).approve(alchemist, 0);
            IERC20(token).approve(alchemist, amount);

            // Only burn if there are deposits
            amountBurned = deposits.length > 0 ? IAlchemistV2(alchemist).burn(amount, recipient) : 0;
        }
```

Changes for `depositIntoRewardPool` function of contract `RewardPoolManager` are as below:

```diff
function depositIntoRewardPool(uint256 _amount) external returns (bool) {
        require(msg.sender == veALCX, "must be veALCX");

+	     IERC20(poolToken).approve(rewardPool, 0);
        IERC20(poolToken).approve(rewardPool, _amount);
        IRewardPool4626(rewardPool).deposit(_amount, address(this));
        return true;
    }
```

## References

<https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/RewardPoolManager.sol#L87>

<https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/RevenueHandler.sol#L210>

## Proof of Concept

The `USDT` [contract](https://etherscan.io/address/0xdac17f958d2ee523a2206206994597c13d831ec7#code#L199) clearly mentions `approve` function to be used for changing allowance only after resetting the approved amount to zero.

```
// To change the approve amount you first have to reduce the addresses`
//  allowance to zero by calling `approve(_spender, 0)` if it is not
//  already 0
```


---

# Agent Instructions: Querying This Documentation

If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question.

Perform an HTTP GET request on the current page URL with the `ask` query parameter:

```
GET https://reports.immunefi.com/alchemix/31523-sc-low-usdt-approval-will-cause-function-failure.md?ask=<question>
```

The question should be specific, self-contained, and written in natural language.
The response will contain a direct answer to the question and relevant excerpts and sources from the documentation.

Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.
