code-423n4 / 2023-11-zetachain-findings

0 stars 0 forks source link

"attack" contract that could exploit the state of the ERC20Custody contract #198

Closed c4-bot-9 closed 11 months ago

c4-bot-9 commented 12 months ago

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/protocol-contracts/contracts/evm/ERC20Custody.sol#L165

Vulnerability details

Impacted

Consider the possibility of an "attack" contract that could exploit the state of the ERC20Custody contract or manipulate its internal accounting.

In the example below, the MaliciousToken contract has a function triggerMaliciousAction that first behaves normally by minting tokens and depositing them into the ERC20Custody contract. It then performs a malicious action by artificially inflating the balance of the custody contract. This could disrupt the internal accounting of the ERC20Custody contract.

Preventing such attacks would require additional safeguards in the ERC20Custody contract to validate the integrity of the tokens being deposited.

Proof of Concept

An example of an "attack" contract:

// SPDX-License-Identifier: MIT pragma solidity 0.8.7;

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";

contract MaliciousToken is ERC20 { ERC20Custody public custodyContract;

constructor(address _custodyContract) ERC20("Malicious Token", "MTKN") {
    custodyContract = ERC20Custody(_custodyContract);
}

function triggerMaliciousAction(uint256 amount) public {
    // Simulate normal behavior
    _mint(msg.sender, amount);

    // Deposit tokens into the custody contract
    approve(address(custodyContract), amount);
    custodyContract.deposit(abi.encodePacked(msg.sender), this, amount, "Normal deposit");

    // Malicious action: artificially inflate the balance or trigger unexpected logic
    _mint(address(custodyContract), amount * 100); // Inflating the balance
    // Other unexpected logic can be added here
}

}

To implement the necessary checks in the ERC20Custody contract, you can modify the deposit function to include a verification step that compares the expected and actual token amounts received by the contract. Here's an example of how this can be done:

function deposit( bytes calldata recipient, IERC20 asset, uint256 amount, bytes calldata message ) external nonReentrant { if (paused) { revert IsPaused(); } if (!whitelisted[asset]) { revert NotWhitelisted(); } if (zetaFee != 0 && address(zeta) != address(0)) { zeta.safeTransferFrom(msg.sender, TSSAddress, zetaFee); }

uint256 oldBalance = asset.balanceOf(address(this));
asset.safeTransferFrom(msg.sender, address(this), amount);
uint256 newBalance = asset.balanceOf(address(this));
uint256 actualReceivedAmount = newBalance - oldBalance;

// Verify that the actual received amount matches the expected amount
require(actualReceivedAmount == amount, "Unexpected token amount received");

emit Deposited(recipient, asset, actualReceivedAmount, message);

}

In this modified function: This line records the balance of tokens held by the contract before the deposit transaction. The oldBalance variable stores the contract's token balance before the transfer. uint256 oldBalance = asset.balanceOf(address(this));

This line executes the token transfer from the sender to the contract. The newBalance variable stores the contract's token balance after the transfer. The actualReceivedAmount is calculated by subtracting the oldBalance from the newBalance. asset.safeTransferFrom(msg.sender, address(this), amount);

This line records the new balance of tokens after the transfer. uint256 newBalance = asset.balanceOf(address(this));

This line calculates the actual amount of tokens received by the contract. This is crucial because some ERC20 tokens might have transfer fees or other mechanisms that change the amount received. uint256 actualReceivedAmount = newBalance - oldBalance;

The check require(actualReceivedAmount == amount, "Unexpected token amount received"); ensures that the contract received the expected amount of tokens. This check is essential to guard against tokens that have built-in transfer fees or other behaviors that could disrupt the expected flow of the deposit function.

A require statement checks if the actualReceivedAmount matches the amount specified in the deposit call. If the amounts don't match, the function reverts, preventing any further execution and protecting against unexpected token behavior.

Tools Used

VS code

Recommended Mitigation Steps

This test script is designed to evaluate the security of the ERC20Custody contract against an exploit attempt by a MaliciousToken. The test follows these steps:

const { expect } = require("chai"); const { ethers } = require("hardhat");

describe("ERC20Custody and MaliciousToken Exploit Test", function () { let ERC20Custody, MaliciousToken, custody, maliciousToken, owner, attacker;

beforeEach(async function () { [owner, attacker] = await ethers.getSigners();

ERC20Custody = await ethers.getContractFactory("ERC20Custody");
custody = await ERC20Custody.deploy(/* constructor arguments */);
await custody.deployed();

MaliciousToken = await ethers.getContractFactory("MaliciousToken");
maliciousToken = await MaliciousToken.deploy(custody.address);
await maliciousToken.deployed();

});

it("should exploit ERC20Custody via MaliciousToken", async function () { // Simulate a legitimate interaction await maliciousToken.connect(attacker).triggerMaliciousAction();

// Assertions to verify if the exploit succeeded
// Replace with the actual state or balance checks as per your contract's logic
const manipulatedState = await custody.manipulatedState(); // example state check
expect(manipulatedState).to.be.true;

}); });

Setup: Initializes the testing environment, deploying both the ERC20Custody and MaliciousToken contracts, and assigns roles to different participants (owner and attacker).

Exploit Simulation: The test simulates an attack where the MaliciousToken attempts to exploit a vulnerability in the ERC20Custody contract. This is done by calling a function in MaliciousToken (e.g., triggerMaliciousAction) that is designed to perform the exploit.

Assertion: After the exploit attempt, the test checks the state of the ERC20Custody contract to verify whether the attack was successful. This could involve checking balances, internal states, or other relevant contract properties. The test expects a certain condition to be true if the exploit succeeded.

The primary goal of this test is to verify the robustness of the ERC20Custody contract against specific types of attacks, ensuring its security and reliability in handling ERC20 tokens, especially when interacting with potentially malicious tokens.

Assessed type

Access Control

c4-pre-sort commented 11 months ago

DadeKuma marked the issue as insufficient quality report

c4-judge commented 11 months ago

0xean marked the issue as unsatisfactory: Invalid