sherlock-audit / 2024-04-titles-judging

6 stars 6 forks source link

0x77 - incorrect implementation of refund mechanism #301

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 3 months ago

0x77

medium

incorrect implementation of refund mechanism

Summary

_refundExcess function is implemented in edition.sol whereas edition.sol's functions are forwarding all of the msg.value to the FeeManager.sol contract. User never get the excess funds back.

Vulnerability Detail

The _refundExcess function is currently implemented in the edition.sol. Any payable function of the edition.sol contract is forwarding all the native coin (msg.value) to FeeManager .sol contract. There is no mechanism in FeeManager.sol that returns the left over msg.value back to the edition.sol contract. there is no use of implementing _refundExcess function in the edition.sol contract.

Impact

user will never get their excess of native coin back.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/editions/Edition.sol#L512

Tool used

Manual Review

Recommendation

there are ways to handle this

  1. implement _refundExcess function in the FeeManager.sol
  2. implement a require statement which let the users only send the msg.value equals to the amount required by the function in edition.sol contract.
  3. return the amount back to the edition contract from feeManager after processing. Note( the refund excess function also has a vulnerability which is submitted as a separate audit item).