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

Insecure ERC20 Operations

sauronsol

Description

The TokenDivider contract uses insecure ERC20 operations (transfer and transferFrom) without checking their return value. Some ERC20 tokens may return false instead of revert in case of failure, which can lead to state inconsistencies.

Code Location:

bool transferSuccess = IERC20(erc20).transfer(msg.sender, amount);
IERC20(tokenInfo.erc20Address).transferFrom(msg.sender, to, amount);
IERC20(tokenInfo.erc20Address).transferFrom(from, to, amount);
IERC20(order.erc20Address).transfer(msg.sender, order.amount);

Impact

Severity: Medium

If a non-compliant ERC20 token is used (returning false instead of revert), transfer operations can silently fail, leading to:

  • Fund losses

  • State inconsistencies

  • Partially executed transactions

Proof of Concept

A test was created to demonstrate the potential issue:

function testUnsafeErc20Operations() public {
// Configure a malicious ERC20 token that returns false
maliciousToken.setReturnFalse(true);
// The transfer returns false but does not revert
bool success = maliciousToken.transfer(BUYER, AMOUNT);
assertFalse(success);
// Balances have not changed despite silent failure
assertEq(maliciousToken.balanceOf(USER), AMOUNT);
assertEq(maliciousToken.balanceOf(BUYER), 0);
}

Tools Used

  • Manual code analysis

  • Foundry tests

  • Aderyn (static analysis tool)

Recommendations

  1. Use the SafeERC20 library from OpenZeppelin:

import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract TokenDivider is IERC721Receiver, Ownable {
using SafeERC20 for IERC20;
// Replace insecure calls with their secure equivalents
IERC20(erc20).safeTransfer(msg.sender, amount);
IERC20(tokenInfo.erc20Address).safeTransferFrom(msg.sender, to, amount);
}

  1. Or manually check returns and revert in case of failure:

bool success = IERC20(erc20).transfer(msg.sender, amount);
require(success, "ERC20 transfer failed");
Updates

Lead Judging Commences

fishy Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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