Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Denial of Service in settleAsMaker and settleAskTaker Functions Due to Missing msg.value in Payable Function Call

pelz

Summary

The settleAsMaker and settleAskTaker functions in the DeliveryPlace.sol contract call the TokenManager::tillIn function, which is payable. However, these functions do not pass the required msg.value when the token being settled is the native token (e.g., ETH). This omission will cause the transaction to revert, resulting in a denial of service (DoS) for users dealing with native tokens.

Vulnerability Details

Location: DeliveryPlace.sol

Description:

The settleAsMaker and settleAskTaker functions handle the settlement of assets for a maker and a taker, respectively. They both make an external call to the TokenManager::tillIn function, which is payable. The tillIn function performs a check to ensure that the msg.value sent with the call matches the _amount being deposited when the token is a native token (e.g., ETH). If msg.value is not provided or is less than the required amount, the transaction reverts.

if (_tokenAddress == wrappedNativeToken) {
if (msg.value < _amount) {
revert Errors.NotEnoughMsgValue(msg.value, _amount);
}
IWrappedNativeToken(wrappedNativeToken).deposit{value: _amount}();
_safe_transfer(wrappedNativeToken, capitalPoolAddr, _amount);
}

The issue arises because the settleAsMaker and settleAskTaker functions do not pass msg.value when calling tillIn, which will cause the transaction to revert if the token is a native token. This results in a denial of service (DoS) for any user attempting to settle with a native token.

Proof of Concept

Here is the relevant code from the settleAsMaker function:

if (settledPointTokenAmount > 0) {
tokenManager.tillIn( // @audit payable function, if tokenAddress is native it'll revert.
_msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
}

Since msg.value is not passed when tillIn is called from settleAsMaker, if the marketPlaceInfo.tokenAddress is the native token (e.g., ETH), the transaction will revert due to the NotEnoughMsgValue error.

The below test function proves this:

function test_ask_offer_protected_eth_revert() public {
vm.startPrank(user);
​
preMarktes.createOffer{value: 0.012 * 1e18}(
CreateOfferParams(
marketPlace,
address(weth9),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker{value: 0.005175 * 1e18}(offerAddr, 500);
​
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer{value: 0.0072 * 1e18}(
stock1Addr,
0.006 * 1e18,
12000
);
​
address offer1Addr = GenerateAddress.generateOfferAddress(1);
preMarktes.closeOffer(stock1Addr, offer1Addr);
preMarktes.relistOffer{value: 0.0072 * 1e18}(stock1Addr, offer1Addr);
​
vm.stopPrank();
​
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(weth9),
0.01 * 1e18,
block.timestamp - 1,
3600
);
​
// call would revert with NotEnoughMsgValue
vm.expectRevert();
vm.startPrank(user);
deliveryPlace.settleAskMaker(offerAddr, 500);
vm.stopPrank();
}

Root Cause:

The root cause is the missing msg.value parameter when calling the tillIn function in the settleAsMaker and settleAskTaker functions, which causes the transaction to revert if the native token is used.

Impact

High: This issue results in a denial of service (DoS) for users attempting to settle trades involving native tokens, preventing them from completing their transactions.

Tools Used

  • Manual Review

Recommendations

. Pass msg.value When Calling tillIn: Modify the settleAsMaker and settleAskTaker functions to pass msg.value when calling the tillIn function. Ensure that the correct value is passed based on the token type.

Example fix for settleAsMaker:

if (settledPointTokenAmount > 0) {
if (marketPlaceInfo.tokenAddress == wrappedNativeToken) {
tokenManager.tillIn{value: settledPointTokenAmount}(
_msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
} else {
tokenManager.tillIn(
_msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
}
}
Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-PreMarkets-settleAskMaker-settleAskTaker-no-msg.value-sent

Invalid, in `settleAskMaker` and `settleAskTaker` you are settling the point token to be given to the takers, which is an ERC20 token, so no native ETH is involved and thus no msg.value is required.

Support

FAQs

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