Submission Details
Severity:
Improper ETH Management in `buyOrder()` function
Vulnerability Details
In the current logic of the buyOrder function, there is no logic to handle scenarios where users overpay for an order. This will always happen because users may not be aware of the actual value of an order after fees have been added.
Impact
Users end up overpaying for an order.
Tools Used
Manual Review
PoC
function test_can_overpay_for_order() public nftDivided {
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
// USER creates sell order for and specifies the price as 1e18 for each token piece
vm.startPrank(USER);
erc20Mock.approve(address(tokenDivider), AMOUNT);
tokenDivider.sellErc20(address(erc721Mock), 1e18, AMOUNT);
vm.stopPrank();
// USER2 overpays for order because
vm.prank(USER2);
tokenDivider.buyOrder{value: 7e18}(0, USER);
}
Recommendations
Refactor the
buyOrder()to check that themsg.valueis == price plus seller fee.
function buyOrder(uint256 orderIndex, address seller) external payable nonReentrant {
...
...
uint256 fee = order.price / 100;
uint256 sellerFee = fee / 2;
- if (msg.value < order.price + sellerFee) {
+ if (msg.value == order.price + sellerFee) {
revert TokenDivider__InsuficientEtherForFees();
}
...
...
}
There should be an off-chain notifier that displays the total amount to pay for an order.
Updates
Lead Judging Commences
fishy 7 months ago
Submission Judgement Published Assigned finding tags:
Token misshandling
The extra eth sent by the user in the buy order will be locked in the contract forever
Rewards breakdown
nSLOC
181This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.