Pieces Protocol

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

User can sell their tokens with price `0` and another can buy it at price `0`

Description

In `ToeknDivider::sellErc20` function there is no checks for `price` weather it is greater than `zero` or not.
```javascript
function sellErc20(address nftPegged, uint256 price,uint256 amount) external {
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);
IERC20(tokenInfo.erc20Address).transferFrom(msg.sender,address(this), amount);
}
```
In this function peremeter `price` is passed which denotes the price of the selling token which is set by the seller. but the price can be set to `zore` as any checks is not present. Dur to this issue anyone can buy seller's token with `zero` price also contract won't be able to get any `fees` for transaction.

Impact

User can buy anyone's token with `zero` price and contract won't get any fees for that.

Proof of Concept

Add this into your `TokenDividerTest.t.sol`.
Code:
```javascript
function testZeroAmountCanbeSell() public nftDivided {
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
vm.startPrank(USER);
erc20Mock.approve(address(tokenDivider), AMOUNT);
tokenDivider.sellErc20(address(erc721Mock), 0, AMOUNT);
vm.stopPrank();
console.log("Price of order: ", tokenDivider.getOrderPrice(USER, 0));
address user1 = makeAddr("user1");
vm.deal(user1, 1 ether);
vm.prank(user1);
tokenDivider.buyOrder{value: 0}(0, USER);
console.log("Balace of user1: ", tokenDivider.getBalanceOf(user1, address(erc20Mock)));
}
```

Recommended Mitigation

Add the checks for the price value in `sellErc20` function as suggested below.
```diff
function sellErc20(address nftPegged, uint256 price,uint256 amount) external {
if(nftPegged == address(0)) {
revert TokenDivider__NftAddressIsZero();
}
if( amount == 0) {
revert TokenDivider__AmountCantBeZero();
}
+ if( price == 0) {
+ revert TokenDivider__PriceCantBeZero();
+ }
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);
IERC20(tokenInfo.erc20Address).transferFrom(msg.sender,address(this), amount);
}
```
Also add the custom error into the `TokenDivider` contract
```diff
+ error TokenDivider__PriceCantBeZero();
```
Updates

Lead Judging Commences

fishy Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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