sherlock-audit / 2023-12-avail-judging

4 stars 4 forks source link

0xlamide - Overpayed fees are not refunded to user resulting in loss of funds for the user #44

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

0xlamide

medium

Overpayed fees are not refunded to user resulting in loss of funds for the user

Summary

In the AvailBalance.sol::sendMessage(), when user send more than the fees, the remaining eth after fees is not refunded to the user

Vulnerability Detail

AvailBalance.sol::sendMessage() accepts fees in native ETH, but does not return overpayments of the fees to the user. Overpayments are likely to occur since the function only checks if msg.value is lesser than the fee. https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L306-L308 However, the function does not refunds the user in the case of overpayment, instead the overpayment account accrue in the contract balance

Scenerio: Alice calls the AvailBalance::sendMessage() and sends 0.9 eth as intended fee

The getfee calculates 0.5eth to be fee and the function does not revert as msg.value is greater that 0.5 eth

The remaining 0.4eth is not refunded to the user and it accrues on the contract balance

Impact

overpayed fees are not refunded to user which result in loss of eth for the user

Code Snippet

https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L300-L321 https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L306-L308

Tool used

Manual Review

Recommendation

Calculate and refund overpayment amounts to callers. This can be done by removing fee from msg.sender and after message is sent, refunded the user with the remaining amount

sherlock-admin commented 8 months ago

2 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Invalid. User mistake. See Sherlock documentation

takarez commented:

valid because { valid and a duplicate of issue 013}