Pieces Protocol
First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Inconsistent Token Balances Due to Direct ERC20 Transfers

francohacker

Summary

A vulnerability has been identified in the TokenDivider contract where transferring ERC20 tokens directly using the transfer method of the ERC20 contract can lead to inconsistencies in the token balances maintained by the TokenDivider contract. This issue arises because the TokenDivider contract does not intercept or account for direct ERC20 transfers, leading to potential discrepancies in the recorded balances.

Vulnerability Details

The TokenDivider contract is designed to manage the division and transfer of NFTs into ERC20 tokens. It maintains a mapping called balances to track the ERC20 token balances of users. The contract provides a method transferErcTokens to securely transfer these tokens and update the balances mapping accordingly.

However, users can still transfer ERC20 tokens directly using the transfer method of the ERC20 contract. When this happens, the TokenDivider contract is not aware of the transfer, and the balances mapping is not updated. This can lead to a situation where the TokenDivider contract's recorded balances do not match the actual balances of the ERC20 tokens, causing inconsistencies and potential security issues.

Steps to Reproduce

  1. Setup: Deploy the TokenDivider contract and an ERC20 token contract.

  2. Divide NFT: Use the divideNft method to divide an NFT into ERC20 tokens.

  3. Direct Transfer: Transfer the ERC20 tokens directly using the transfer method of the ERC20 contract.

  4. Check Balances: Compare the balances recorded in the TokenDivider contract with the actual balances of the ERC20 tokens.

Example Code

Here is an example of how the vulnerability can be exploited:

// TokenDividerTest.t.sol
function testDirectErc20Transfer() public {
vm.startPrank(USER);
erc721Mock.approve(address(tokenDivider), TOKEN_ID);
tokenDivider.divideNft(address(erc721Mock), TOKEN_ID, AMOUNT);
vm.stopPrank();
​
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
vm.prank(USER);
bool success = erc20Mock.transfer(USER2, AMOUNT);
if (!success) {
console.log("Transfer failed");
}
​
// Check balances
uint256 recordedBalance = tokenDivider.getBalanceOf(USER, address(erc20Mock));
uint256 actualBalance = erc20Mock.balanceOf(USER);
assertEq(recordedBalance, actualBalance, "Recorded balance does not match actual balance");
}

Impact

The impact of this vulnerability is significant as it can lead to:

  • Inconsistent Balances: The TokenDivider contract's recorded balances may not match the actual ERC20 token balances.

  • Security Risks: Users may exploit this inconsistency to manipulate balances and withdraw more tokens than they actually own.

  • Operational Issues: Functions that rely on the balances mapping for validation may fail or behave unexpectedly.

Tools

manual review, github copilote

Recommendations

To mitigate this vulnerability, consider the following approaches:

  1. Restrict Direct Transfers: Modify the ERC20 contract to restrict direct transfers and only allow transfers through the TokenDivider contract.

  2. Intercept Transfers: Implement a mechanism in the TokenDivider contract to intercept and handle direct ERC20 transfers, ensuring the balances mapping is updated accordingly.

Suggested Code Changes

Here is an example of how to restrict direct transfers in the ERC20 contract:

// ERC20Mock.sol
contract ERC20Mock is ERC20 {
address public tokenDivider;
​
constructor(string memory name, string memory symbol, address _tokenDivider) ERC20(name, symbol) {
tokenDivider = _tokenDivider;
}
​
function transfer(address recipient, uint256 amount) public override returns (bool) {
require(msg.sender == tokenDivider, "Transfers can only be made through TokenDivider");
return super.transfer(recipient, amount);
}
​
function transferFrom(address sender, address recipient, uint256 amount) public override returns (bool) {
require(msg.sender == tokenDivider, "Transfers can only be made through TokenDivider");
return super.transferFrom(sender, recipient, amount);
}
}

By implementing these changes, you can ensure that all ERC20 token transfers are properly accounted for in the TokenDivider contract, maintaining consistency and security

Updates

Lead Judging Commences

fishy Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Transfer ERC20ToGenerateNftFraccion separately to the contract

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.