Pieces Protocol

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

[M-8]: Denial of Service (DoS) in `TokenDivider::sellErc20` Due to Non-Compliant ERC20 Tokens

Summary

The sellErc20 function is vulnerable to Denial of Service (DoS) attacks because it relies on the transferFrom function of an ERC20 token to transfer tokens to the contract. If the transferFrom call fails (e.g., due to a non-compliant ERC20 token that returns false instead of reverting), the function will not revert, and the sell order will remain in an inconsistent state. This could make the function unusable for certain tokens.

The issue lies in the following section of the sellErc20 function:

function sellErc20(address nftPegged, uint256 price, uint256 amount) external {
// ...
IERC20(tokenInfo.erc20Address).transferFrom(msg.sender, address(this), amount); // No revert on failure
// ...
}

Vulnerability Details

The sellErc20 function relies on the transferFrom function of an ERC20 token to transfer tokens to the contract. However, it does not check the return value of transferFrom or handle failures properly. If the transferFrom call fails (e.g., due to a non-compliant ERC20 token that returns false instead of reverting), the function will not revert, and the sell order will remain in an inconsistent state. This can lead to a Denial of Service (DoS) attack, where the function becomes unusable for certain tokens, and the contract state (e.g., balances and sell orders) may become corrupted.

Impact

Medium Impact: If the transferFrom call fails, the sell order will be created, but the tokens will not be transferred to the contract. This leaves the contract in an inconsistent state and could prevent users from creating valid sell orders.

Users may lose trust in the platform if they cannot reliably create sell orders.

Tools Used

Manual Review, Foundry

Recommendations

To prevent DoS attacks, add a mechanism to handle non-compliant tokens or allow users to cancel orders. Here’s an example of how to implement this:

  1. Check the Return Value of transferFrom:

Ensure the function reverts if the transferFrom call fails.

  1. Allow Users to Cancel Orders:

Add a function to cancel sell orders if the transferFrom call fails.

function sellErc20(address nftPegged, uint256 price, uint256 amount) external {
// Existing checks...
if (nftPegged == address(0)) {
revert TokenDivider__NftAddressIsZero();
}
if (amount == 0) {
revert TokenDivider__AmountCantBeZero();
}
ERC20Info memory tokenInfo = nftToErc20Info[nftPegged];
if (balances[msg.sender][tokenInfo.erc20Address] < amount) {
revert TokenDivider__InsuficientBalance();
}
balances[msg.sender][tokenInfo.erc20Address] -= amount;
s_userToSellOrders[msg.sender].push(
SellOrder({
seller: msg.sender,
erc20Address: tokenInfo.erc20Address,
price: price,
amount: amount
})
);
emit OrderPublished(amount, msg.sender, nftPegged);
// Check the return value of transferFrom and revert if it fails
bool success = IERC20(tokenInfo.erc20Address).transferFrom(msg.sender, address(this), amount);
if (!success) {
revert TokenDivider__TransferFailed();
}
}
function cancelSellOrder(uint256 orderIndex) external {
SellOrder memory order = s_userToSellOrders[msg.sender][orderIndex];
// Remove the order from the array
s_userToSellOrders[msg.sender][orderIndex] = s_userToSellOrders[msg.sender][s_userToSellOrders[msg.sender].length - 1];
s_userToSellOrders[msg.sender].pop();
// Return the tokens to the seller
bool success = IERC20(order.erc20Address).transfer(msg.sender, order.amount);
require(success, "TokenDivider: Transfer failed");
}

The return value check ensures that the function reverts if the transferFrom call fails, preventing inconsistent states.

The cancel order mechanism allows users to recover their tokens if the transferFrom call fails, improving the user experience.

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.