sherlock-audit / 2023-04-jojo-judging

7 stars 4 forks source link

Avci - the Funding.sol contract:executeWithdraw function has some problems in logic #475

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Avci

medium

the Funding.sol contract:executeWithdraw function has some problems in logic

Summary

the Funding.sol contract:executeWithdraw function has some problems in logic in primaryAmount and pendingPrimaryWithdraw same in secondaryAmount and secondaryPrimaryWithdraw

Vulnerability Detail

in line 111 we can see the uint256 primaryAmount = state.pendingPrimaryWithdraw[msg.sender]; then it makes state.pendingPrimaryWithdraw[msg.sender] = 0;

Impact

basically, if user wants to Withdraw primary again for a second time they will face error in withdrawing because logic cannot transfer 0 amount because primaryAmount set to 0 second time

Code Snippet

   function executeWithdraw(
        Types.State storage state,
        address to,
        bool isInternal
    ) external {
        require(
            state.withdrawExecutionTimestamp[msg.sender] <= block.timestamp,
            Errors.WITHDRAW_PENDING
        );
        uint256 primaryAmount = state.pendingPrimaryWithdraw[msg.sender];
        uint256 secondaryAmount = state.pendingSecondaryWithdraw[msg.sender];
        state.pendingPrimaryWithdraw[msg.sender] = 0;
        state.pendingSecondaryWithdraw[msg.sender] = 0;
        // No need to change withdrawExecutionTimestamp, because we set pending
        // withdraw amount to 0.
        _withdraw(
            state,
            msg.sender,
            to,
            primaryAmount,
            secondaryAmount,
            isInternal
        );
    }

https://github.com/sherlock-audit/2023-04-jojo/blob/main/smart-contract-EVM/contracts/lib/Funding.sol#L102-L126

Tool used

Manual Review

Recommendation

consider setting it to the amount that the user didn't withdraw yet