code-423n4 / 2022-08-foundation-findings

0 stars 0 forks source link

Possible lost msg.value #286

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-08-foundation/blob/main/contracts/mixins/shared/FETHNode.sol#L46-L63

Vulnerability details

Impact

Possible lost value in _tryUseFETHBalance function call

Proof of Concept

In call _tryUseFETHBalance can send value and the asset can be an ERC20(!= address(0)). if send as asset ETH(== address(0)) and send more value than totalAmount(totalAmount > msg.value), the exedent will lost.

Tools Used

Recommended Mitigation Steps

In FETHNode, _tryUseFETHBalance function:

Check if the msg.value is zero when the _asset is ERC20(!= address(0)) Check if the msg.value is equal to _amount when the _asset ETH(== address(0))

as seen in previous contest: https://github.com/code-423n4/code423n4.com/blob/main/_data/reports/2022-05-sturdy.md#m-01-possible-lost-msgvalue

HardlyDifficult commented 1 year ago

Invalid. The caller here will confirm that msg.value is not greater than what was expected.

HickupHH3 commented 1 year ago

Agree with sponsor, excess cannot be sent.