sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

0x486776 - Reentrancy attack by edition referrer. #189

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

0x486776

high

Reentrancy attack by edition referrer.

Summary

Assigning a malicious contract as the referrer of a new edition can enable a reentrancy attack on any minting process.

Vulnerability Detail

Consider the following scenario:

  1. Alice creates a new edition and sets the referrer to following malicious contract.
    contract Referrer {

        Edition edition = Alice's edition; // the edition Alice created

        receive() external payable {

            address to_ = address(1);
            uint256[] calldata tokenIds_ = new uint256[](0);
            uint256[] calldata amounts_ = new uint256[](0);
            bytes calldata data_ = "";

            edition.mintBatch{value: 1 wei}(
                to_, tokenIds_, amounts_, data_
            );
        }
    }
  1. Bob mints at Alice's editin. Then Alice's referrer receives some fees. The referrer then calls edition::mintBatch with empty parameters. Subsequently, edition::mintBatch mints nothing and triggers _refundExcess() at L296.
    function mintBatch(
        address to_,
        uint256[] calldata tokenIds_,
        uint256[] calldata amounts_,
        bytes calldata data_
    ) external payable {
        for (uint256 i = 0; i < tokenIds_.length; i++) {
            Work storage work = works[tokenIds_[i]];

            // wake-disable-next-line reentrancy
            FEE_MANAGER.collectMintFee{value: msg.value}(
                this, tokenIds_[i], amounts_[i], msg.sender, address(0), work.strategy
            );

            _checkTime(work.opensAt, work.closesAt);
            _updateSupply(work, amounts_[i]);
        }

        _batchMint(to_, tokenIds_, amounts_, data_);
296     _refundExcess();
    }
  1. edition::_refundExcess proceeds to transfer all balances, including Bob's remaining ether, to Alice's referrer.
    function _refundExcess() internal {
        if (msg.value > 0 && address(this).balance > 0) {
            msg.sender.safeTransferETH(address(this).balance);
        }
    }

Consequently, Bob's remaining Ether is diverted to Alice's referrer, rather than returned to Bob.

Impact

The edition creator can potentially execute reentrancy attacks by assigning a malicious contract as the referrer of the edition.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Edition.sol should inherit the ReentrancyGuard contract.

Duplicate of #268

whitehair0330 commented 4 months ago

Escalate Dup of 268

sherlock-admin3 commented 4 months ago

Escalate Dup of 268

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

WangSecurity commented 4 months ago

Agree with the escalation, planning to accept and duplicate with #268

Evert0x commented 4 months ago

Result: High Duplicate of #268

sherlock-admin4 commented 4 months ago

Escalations have been resolved successfully!

Escalation status: