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

There is no way to track sent msg.value which can lead to stuck ETH

coditoorgeneral

Summary

The function TokenManager.sol::tillIn allows msg.value be used, the tillIn converts eth amount to weth and then transferred as seen here

if (_tokenAddress == wrappedNativeToken) {
/**
* @dev token is native token
* @notice check msg value
* @dev if msg value is less than _amount, revert
* @dev wrap native token and transfer to capital pool
*/
if (msg.value < _amount) {
revert Errors.NotEnoughMsgValue(msg.value, _amount);
}
IWrappedNativeToken(wrappedNativeToken).deposit{value: _amount}();
_safe_transfer(wrappedNativeToken, capitalPoolAddr, _amount);

Vulnerability Details

The TokenManager.sol::tillIn receives msg.value, and does the following things as will be highlighted

  • verifies msg.value < _amount

  • wrap eth to weth

  • transfer weth to capitalPoolAddr

But a scenario not considered is when msg.value is significantly greater than amount, scenarios like this are likely to happen.

In the function Premarkets.sol:createOffer https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L39 , the tillIn function called with msg.value

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

The disconnect between the amount used in the CreateOfferParams.amount and msg.value, could bring up scenarios where msg.value > params.amount and lead to loss of funds that increases significantly as the difference increses significantly.

Although the lost ETH rescuable, it leads to owner calls to rescueToken and distributed rescued ETH to affected users causing some loss of funds still and hence why the impact is a medium impact but highly likely.

Impact

Loss of funds to a certain degree

Tools Used

Manual Review

Recommendations

  • ensure msg.value == _amount

  • deposit IWrappedNativeToken(wrappedNativeToken).deposit{value: msg.value}(); and override amount with msg.value

Updates

Lead Judging Commences

0xnevi Lead Judge 12 months 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.