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

Inconsistent payment handling in listOffer() which can lead to loss of funds

sabit

Summary

The listOffer() function is marked as payable, allowing it to receive ETH. However, the function only requires payment in specific circumstances, which could lead to unexpected behavior and potential loss of funds.

Vulnerability Details

The listOffer() function is marked as payable.
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L299

Token transfer only occurs when the offer settlement type is Protected:

if (makerInfo.offerSettleType == OfferSettleType.Protected) {
uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
_amount,
true,
Math.Rounding.Ceil
);

ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.tillIn{value: msg.value}(
_msgSender(),
makerInfo.tokenAddress,
transferAmount,
false
);
}

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L346C6-L362C10

For other settlement types (e.g., Turbo), no token transfer occurs, but the function can still receive ETH.

Users may send ETH unnecessarily when listing offers with non-Protected settlement types. Sent ETH in these cases would be locked in the contract without a clear mechanism for retrieval.

Proof of Concept:

  1. A user creates an offer with a Turbo settlement type.

  2. The same user calls listOffer() and sends ETH along with the transaction.

  3. The function executes successfully, but the sent ETH is not used and becomes locked in the contract.

Impact

Users may send ETH unnecessarily when listing offers with non-Protected settlement types and sent token in these cases would be locked in the contract without a clear mechanism for retrieval, causing loss of funds.

Tools Used

Manual review

Recommendations

Add a check at the beginning of listOffer() to revert if msg.value > 0 for non-Protected settlement types.

Updates

Lead Judging Commences

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

[invalid] finding-TokenManager-tillin-excess

Invalid, these are by default, invalid based on codehawks [general guidelines](https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity#findings-that-may-be-invalid). The check implemented is simply a sufficiency check, it is users responsibility to only send an appropriate amount of native tokens where amount == msg.value when native token is intended to be used as collateral (which will subsequently be deposited as wrapped token). All excess ETH can be rescued using the `Rescuable.sol` contract. > Users sending ETH/native tokens > If contracts allow users to send tokens acc111identally.

Support

FAQs

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