sherlock-audit / 2024-02-telcoin-platform-audit-update-judging

3 stars 1 forks source link

Anubis - Inadequate Token Pair Validation in DefiSwap Potentially Leading to Incorrect Balance Calculations #22

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

Anubis

high

Inadequate Token Pair Validation in DefiSwap Potentially Leading to Incorrect Balance Calculations

Summary

The _verifyDefi function exhibits a vulnerability due to insufficient validation of token pairs involved in a decentralized finance (DeFi) swap operation, potentially leading to incorrect balance calculations. While the function validates the presence of a fee token, aggregator, swap data, referrer, and plugin, it fails to verify the legitimacy or protocol compliance of the token pairs being swapped.

Vulnerability Detail

The root cause of the vulnerability "Inadequate Token Pair Validation in DefiSwap Potentially Leading to Incorrect Balance Calculations" in the provided code is that the function _verifyDefi does not properly validate the token pair before performing balance calculations.

Specifically, in lines 213 and 214, the code checks if the feeToken is not equal to TELCOIN and if it is not the zero address. However, it does not verify if the token pair being used in the swap is valid or supported by the DefiSwap contract. This lack of validation can potentially lead to incorrect balance calculations if an unsupported or invalid token pair is used in the swap.

The vulnerability in the code lies in the inadequate validation of the token pair in the DefiSwap function. Specifically, the code does not properly validate the token pair before performing balance calculations, which can potentially lead to incorrect results.

To exploit this vulnerability, an attacker could manipulate the token pair input in a way that the balance calculations are based on incorrect token values. This could result in the attacker gaining more tokens than they should have or causing a loss to the system.

Proof of Concept (PoC) code:

1.Attacker creates a malicious token pair input:

address maliciousToken = 0x1234567890123456789012345678901234567890; // Malicious token address
address legitimateToken = TELCOIN; // Legitimate token address

DefiSwap memory defi = DefiSwap({
    feeToken: maliciousToken,
    aggregator: address(0),
    swapData: "",
    referrer: address(0),
    plugin: address(0)
});
// Call the _verifyDefi function with the malicious token pair
_verifyDefi(legitimateToken, maliciousToken, defi);

2.By passing the malicious token pair to the _verifyDefi function, the attacker can potentially bypass the inadequate token pair validation and manipulate the balance calculations in their favor.

3.This could result in the attacker gaining more tokens or causing a loss to the system due to incorrect balance calculations.

It is important to update the code to include proper validation of the token pair before performing balance calculations to prevent such vulnerabilities in the future.

Impact

This oversight could be exploited by an attacker to conduct swaps with arbitrary token pairs, causing downstream accounting discrepancies and potentially compromising the financial integrity of the protocol.

Code Snippet

https://github.com/sherlock-audit/2024-02-telcoin-platform-audit-update/blob/main/telcoin-contracts/contracts/swap/AmirX.sol#L211-L222

Tool used

Manual Review

Recommendation

The vulnerability in the code is that the token pair validation is inadequate. The code only checks if the fee token is not equal to TELCOIN and if the fee token address is not equal to 0. However, it does not verify if the fee token is a valid token pair for the swap. This can potentially lead to incorrect balance calculations or other issues in the DefiSwap functionality.

To fix this issue, we need to add a validation step to ensure that the token pair is valid for the swap. This can be done by checking if both tokens in the pair are valid ERC20 tokens. We can achieve this by using the ERC20 interface to verify the tokens.

Here is an example of how the code can be patched to include token pair validation:

// Import ERC20 interface
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

function _verifyDefi(address tokenA, address tokenB, DefiSwap memory defi) internal view {
    // validate pathway
    if (defi.feeToken != TELCOIN && address(defi.feeToken) != address(0)) {
        if (defi.aggregator == address(0) || defi.swapData.length == 0)
            revert ZeroValueInput("BUYBACK");

        // Validate token pair
        IERC20 tokenAContract = IERC20(tokenA);
        IERC20 tokenBContract = IERC20(tokenB);

        require(tokenAContract.totalSupply() > 0 && tokenBContract.totalSupply() > 0;

    }

    // determines if there is a referrer increase
    if (defi.referrer != address(0)) {
        if (address(defi.plugin) == address(0))
            revert ZeroValueInput("PLUGIN");
    }
}

In this patched code, we have added a validation step to check if both tokens in the token pair are valid ERC20 tokens by calling the totalSupply() function on each token. This ensures that the token pair is valid for the swap and helps prevent incorrect balance calculations.

sherlock-admin4 commented 8 months ago

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

WangAudit commented:

executed by a trusted address -> invalid under Sherlock's rules