c4-bot-2 commented 8 months ago

Lines of code

Vulnerability details

Comments

The original implementation deployed teamVestingWallet with upkeep as the benificiary. It was intended to release team SALT reward from teamVestingWallet to upkeep linely and transfer it to the designated wallet address. However it didn't notice that there is no access control on teamVestingWallet#release(). Anyone can call it to distribute SALT without informing upkeep and make it stucked in upkeep.

Mitigation

commit 5766592 The mitigation deploys teamVestingWallet with teamWallet as the benificiary directly. teamVestingWallet#release() can be called by anyone or in upkeep#performKeep() and the reward will be distributed to teamWallet directly. The mitigation resolved the original issue.

Test

Copy below codes to DAO.t.sol and run COVERAGE="yes" NETWORK="sep" forge test -vv --rpc-url RPC_URL --match-test testTeamRewardReleasedCorrectly. Whether teamVestingWallet#release() is called directly or through upkeep#performUpkeep(), the team reward will be distributed directly to teamWallet.

    function testTeamRewardReleasedCorrectly() public {
        uint releasableAmount = teamVestingWallet.releasable(address(salt));
        uint upKeepBalance = salt.balanceOf(address(upkeep));
        uint teamWalletBalance = salt.balanceOf(address(exchangeConfig.teamWallet()));
        //@audit-info a certain amount of SALT is releasable
        assertTrue(releasableAmount != 0);
        //@audit-info there is no SALT in upkeep
        assertEq(upKeepBalance, 0);
        //@audit-info there is no SALT in teamWallet
        assertEq(teamWalletBalance, 0);
        //@audit-info call release() before performUpkeep()
        teamVestingWallet.release(address(salt));

        upKeepBalance = salt.balanceOf(address(upkeep));
        teamWalletBalance = salt.balanceOf(address(exchangeConfig.teamWallet()));
        //@audit-info all releasable SALT has been sent to teamWallet
        assertEq(teamWalletBalance, releasableAmount);

        vm.warp(block.timestamp + 1 weeks );

        uint releasableAmountByUpkeep = teamVestingWallet.releasable(address(salt));
        assertTrue(releasableAmountByUpkeep != 0);
        //@audit-info performUpkeep() call teamVestingWallet.release() to release SALT
        upkeep.performUpkeep();
        teamWalletBalance = salt.balanceOf(address(exchangeConfig.teamWallet()));
        //@audit-info all releasable SALT has been sent to teamWallet
        assertEq(teamWalletBalance, releasableAmount+releasableAmountByUpkeep);
    }

Conclusion

Confirmed

c4-judge commented 8 months ago

Picodes marked the issue as satisfactory