sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

techOptimizor - Intermediate value sent by the caller can be drained via reentrancy by a malicious creator , collectionReferrer or mint referrer #249

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

techOptimizor

medium

Intermediate value sent by the caller can be drained via reentrancy by a malicious creator , collectionReferrer or mint referrer

Summary

Intermediate value sent by the caller can be drained via reentrancy by a malicious creator , collectionReferrer or mint referrer from the current implementation of _refundExcess() on Edition contract

Vulnerability Detail

function _refundExcess() internal {
        if (msg.value > 0 && address(this).balance > 0) {
            msg.sender.safeTransferETH(address(this).balance);
        }
    }

The current implementation of _refundExcess () sends the balance of its address to msg.sender during refund if both rules are met. The probems comes when sending fee to various recipients

 _route(
            Fee({asset: asset_, amount: mintReferrerShare}),
            Target({target: referrer_, chainId: block.chainid}),
            payer_
        );

        _route(
            Fee({asset: asset_, amount: collectionReferrerShare}),
            Target({target: referrer_, chainId: block.chainid}),
            payer_
        );

if the recipients is malicious and implents a fallback it can potentially call back to any mint function with the exact required fee or just 1 wei because of the logic bug in mints the feeManager balance from excesses can be used to pay for mint fee which will trigger the _refundExcess as msg.value is greater than 0 and address(this).balance > 0 from the initial caller value . the malicious recipint gets the refund while the original caller gets nothing.

setting this as meduim because the mints functions contains a bug that wont allow the refund logic to trigger as of current but this is likely to be corrected that the refunds now works then this become high. But now since the excesses are left in the fee manager because of logic bug is still an issue since the call back can mint with just 1 wei if the excesses can cover the fee

Impact

Lose of users funds

Code Snippet

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

Tool used

Manual Review

Recommendation

Calculating the amount meant to refundback

Dyplicate of #268

techOptimizor commented 3 months ago

This report was marked invalid while its a duplicate of xiaoming90 - Excess ETH of the victim can be stolen by malicious external parties due to re-entrancy attack #268

https://github.com/sherlock-audit/2024-04-titles-judging/issues/268

This is basically same reentrancy report by xiaoming90 but was valid in his end and mine was invalid