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

Absence of Token Approval Verification

swissahman

Summary

The transferErcTokens function attempts to execute an ERC20 transferFrom operation without verifying if the contract has sufficient approval to transfer tokens on behalf of the user. This missing validation could lead to failed transactions and inconsistent state.

Vulnerability Detail

Current implementation:

function transferErcTokens(address nftAddress, address to, uint256 amount) external {
// ... other validations ...
​
ERC20Info memory tokenInfo = nftToErc20Info[nftAddress];
​
// State updates
balances[msg.sender][tokenInfo.erc20Address] -= amount;
balances[to][tokenInfo.erc20Address] += amount;
​
// Transfer without approval check
IERC20(tokenInfo.erc20Address).transferFrom(msg.sender, to, amount);
// Could fail if allowance is insufficient
}

Impact

The vulnerability could result in:

  1. Failed transactions with inconsistent state

  2. Permanent loss of internal balance tracking

  3. Gas waste from failed transactions

  4. User funds becoming stuck in the contract

  5. Discrepancy between internal balances and actual token balances

Proof of Concept

contract ApprovalExploitTest {
TokenDivider public divider;
MockERC20 public token;
function demonstrateApprovalIssue() public {
// 1. Setup
address user = address(this);
address recipient = address(0x123);
uint256 amount = 100;
// 2. No approval given
bool transferSuccess = false;
try divider.transferErcTokens(address(token), recipient, amount) {
transferSuccess = true;
} catch {
// Expected to fail
}
require(!transferSuccess, "Transfer should fail without approval");
// 3. Check if internal balances were updated despite failed transfer
uint256 senderBalance = divider.getBalanceOf(user, address(token));
uint256 recipientBalance = divider.getBalanceOf(recipient, address(token));
// Internal balances might be incorrect here
}
}

Attack Scenario

  1. User calls transferErcTokens without first approving the contract

  2. Internal balances are updated

  3. transferFrom fails due to insufficient allowance

  4. Contract state becomes inconsistent with actual token balances

  5. User funds might become locked

Tool Used

Foundry

Recommended Mitigation Steps

  1. Add Approval Validation

function _validateApproval(
address token,
address owner,
uint256 amount
) private view {
uint256 allowance = IERC20(token).allowance(owner, address(this));
require(allowance >= amount, "Insufficient allowance");
}

  1. Implement Safe Transfer Pattern

function transferErcTokens(
address nftAddress,
address to,
uint256 amount
) external nonReentrant {
// Load token info
ERC20Info memory tokenInfo = _validateTokenInfo(nftAddress);
address tokenAddress = tokenInfo.erc20Address;
// Validate approvals and balances
_validateApproval(tokenAddress, msg.sender, amount);
_validateBalance(msg.sender, tokenAddress, amount);
// Execute transfer first
bool success = IERC20(tokenAddress).transferFrom(
msg.sender,
to,
amount
);
require(success, "Transfer failed");
// Update internal balances after successful transfer
_updateBalances(msg.sender, to, tokenAddress, amount);
emit TokensTransferred(msg.sender, to, tokenAddress, amount);
}

  1. Add Recovery Mechanism

function reconcileBalance(
address token,
address account
) external {
uint256 actualBalance = IERC20(token).balanceOf(account);
uint256 recordedBalance = balances[account][token];
if (actualBalance < recordedBalance) {
balances[account][token] = actualBalance;
emit BalanceReconciled(account, token, actualBalance);
}
}

  1. Complete Implementation with Safeguards

contract TokenDivider {
using SafeERC20 for IERC20;
​
function transferErcTokens(
address nftAddress,
address to,
uint256 amount
) external nonReentrant whenNotPaused {
// Validations
_validateInputs(nftAddress, to, amount);
ERC20Info memory tokenInfo = _validateTokenInfo(nftAddress);
address tokenAddress = tokenInfo.erc20Address;
// Check approvals
uint256 currentAllowance = IERC20(tokenAddress).allowance(
msg.sender,
address(this)
);
require(currentAllowance >= amount, "Insufficient allowance");
// Check balances
require(
balances[msg.sender][tokenAddress] >= amount,
"Insufficient balance"
);
// Execute transfer using SafeERC20
IERC20(tokenAddress).safeTransferFrom(msg.sender, to, amount);
// Update balances
balances[msg.sender][tokenAddress] -= amount;
balances[to][tokenAddress] += amount;
emit TokensTransferred(
msg.sender,
to,
tokenAddress,
amount,
block.timestamp
);
}
}

Additional Recommendations

  1. Implement Approval Tracking

mapping(address => mapping(address => uint256)) public approvalUsage;
​
function _trackApprovalUsage(
address token,
address owner,
uint256 amount
) private {
approvalUsage[token][owner] += amount;
}

  1. Add Pre-transfer Checks

modifier validTransfer(address token, uint256 amount) {
require(token != address(0), "Invalid token");
require(amount > 0, "Invalid amount");
require(
IERC20(token).allowance(msg.sender, address(this)) >= amount,
"Insufficient allowance"
);
_;
}

Updates

Lead Judging Commences

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

Support

FAQs

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