Skip to content

Latest commit

 

History

History
93 lines (67 loc) · 4.06 KB

M-03.md

File metadata and controls

93 lines (67 loc) · 4.06 KB
Raw

Proxy address is not validated in distributeByOwner

Summary

The relationship between the proxy address and the contest is not validated in distributeByOwner.

Vulnerability Details

The owner of the ProxyFactory contract is able to distribute tokens from deployed proxies of contest that are not expired. The distributeByOwner function does not validate that the proxy belongs to the given contest, organizer, and implementation, even though the salt is calculated using these values. So, the owner can use the contestId, organizer, and implementation of any expired contest to distribute tokens from a proxy of another contest that has already been deployed.

Thinking in a not malicious way, if the function distributeByOwner is used by a script to perform the distribution of expired contest and the input holds incorrect values for the proxy address it could lead to a bad distribution of the tokens.

Proof of Concept

The following test shows how the owner can distribute tokens from a proxy of a contest that has not expired by using the contestId, organizer, and implementation of an expired contest.

Note: the snippet shows only the relevant code for the test. Full test file can be found here.

    function test_PoC_M03() public {
        bytes32 contestIdA = keccak256("a contest id");
        bytes32 contestIdB = keccak256("another contest id");
        address organizerB = makeAddr("organizerB");
        uint256 closeTime = block.timestamp + 1 weeks;
        bytes memory data = createData();

        vm.startPrank(owner);
        address implementation = address(new Distributor(address(factory), stadium));

        // Owner set contest A.
        factory.setContest(organizer, contestIdA, closeTime, implementation);

        // Fast forward to close time of contest A.
        uint256 expiration = closeTime + 1 weeks;
        vm.warp(expiration);

        // Owner set contest B.
        closeTime = expiration;
        factory.setContest(organizerB, contestIdB, closeTime, implementation);

        // Simulate tokens send to distribution of Contest B.
        address proxyB = factory.getProxyAddress(_calculateSalt(organizerB, contestIdB, implementation), implementation);
        tokenA.mint(proxyB, 1000e18);

        // Organizer B distribute prizes.
        changePrank(organizerB);
        factory.deployProxyAndDistribute(contestIdB, implementation, data);

        //Sponsor sends more tokens to Proxy B.
        changePrank(owner);
        tokenA.mint(proxyB, 1000e18);

        // Assert proxy B balance is 1000e18.
        assertEq(tokenA.balanceOf(proxyB), 1000e18);

        //Distribute tokens of Proxy B with contest A data set.
        factory.distributeByOwner(proxyB, organizer, contestIdA, implementation, data);

        // Assert proxy B was drained.
        assertEq(tokenA.balanceOf(proxyB), 0);
    }

Impact

Medium. Loss of funds when the function distributeByOwner is misused.

Tools Used

None.

Recommendations

Remove proxy parameter from distributeByOwner function and use the salt to get the proxy address. This way, the proxy address is validated and the owner can only distribute tokens from expired contests.

    function distributeByOwner(
-       address proxy,
        address organizer,
        bytes32 contestId,
        address implementation,
        bytes calldata data
    ) public onlyOwner {
-       if (proxy == address(0)) revert ProxyFactory__ProxyAddressCannotBeZero();
        bytes32 salt = _calculateSalt(organizer, contestId, implementation);
        if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();
        // distribute only when it exists and expired
        if (saltToCloseTime[salt] + EXPIRATION_TIME > block.timestamp) revert
        ProxyFactory__ContestIsNotExpired();
+       address proxy = getProxyAddress(salt, implementation);
        _distribute(proxy, data);
    }