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

Ether sent to `PreMarket::listOffer` and `PreMarket::relistOffer` will result in lost of funds for non-protected offer types

0x5chn0uf

Summary

The PreMarkets::listOffer and PreMarkets::relistOffer functions are marked as payable, allowing it to receive Ether. However, the function only uses msg.value when the offer settlement type is Protected.

For all other settlement types, any Ether sent with the transaction is ignored and becomes permanently locked in the PreMarkets contract.

The root cause is that the function is marked as payable but only handles the msg.value in a specific case, without any mechanism to return or reject unexpected Ether in other cases.

Vulnerability Details

Note: In order to not make this finding extra long, the vulnerability will only be detailed for listOffer. However, the root cause is the same for both functions.

function listOffer(/* ... */) external payable { // @audit - Payable, can receive Ether
// ...
if (makerInfo.offerSettleType == OfferSettleType.Turbo) {
// @audit - In this case, msg.value is not handled!
}
​
if (makerInfo.offerSettleType == OfferSettleType.Protected) {
// ...
@> tokenManager.tillIn{value: msg.value}( // @audit - In this case, msg.value is handled
_msgSender(),
makerInfo.tokenAddress,
transferAmount,
false
);
}
​
// ...
}

Proof of Concept:

  • A user wants to list an offer with a Turbo settlement type.

  • The user accidentally sends 1 Ether along with the listOffer transaction.

  • Since the settlement type is not Protected, the Ether is ignored and becomes locked in the contract.

Proof of Code

function test_unprotectedListOfferFunction() public {
// Setup
vm.startPrank(user1);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo // Settle Type is Turbo
)
);
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 500);
​
deal(user1, 1 ether);
address stock1Addr = GenerateAddress.generateStockAddress(1);
​
// User lists offer with 1 Ether sent accidentally
preMarktes.listOffer{value: 1 ether}(stock1Addr, 1e18, 12000);
​
// The call doesn't revert, and the user loses 1 Ether
assert(address(preMarktes).balance == 1 ether);
​
vm.stopPrank();
}

Impact

Users can permanently lose Ether by accidentally sending it when listing offers with non-protected settlement types.

This could lead to significant financial losses, especially if high-value transactions are involved or if the issue is exploited repeatedly.

Tools Used

Manual Review - Testing

Recommendations

Modify the listOffer function to revert if Ether is sent with a non-Protected offer type:

function listOffer(
address _stock,
uint256 _amount,
uint256 _collateralRate
) external payable {
// ...
​
StockInfo storage stockInfo = stockInfoMap[_stock];
OfferInfo storage offerInfo = offerInfoMap[stockInfo.preOffer];
MakerInfo storage makerInfo = makerInfoMap[offerInfo.maker];
​
+ if (makerInfo.offerSettleType != OfferSettleType.Protected && msg.value > 0) {
+ revert UnexpectedEther();
+ }
​
// ...
}

This change will prevent users from accidentally sending Ether when listing non-Protected offers, thus avoiding the potential for locked funds.

Updates

Lead Judging Commences

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!