0x52 - Funding#requestWithdraw uses incorrect withdraw address

[sherlock-admin]

0x52

medium

Funding#requestWithdraw uses incorrect withdraw address

Summary

When requesting a withdraw, msg.sender is used in place of the from address. This means that withdraws cannot be initiated on behalf of other users. This will break integrations that depend on this functionality leading to irretrievable funds.

Vulnerability Detail

Funding.sol#L69-L82

function requestWithdraw(
    Types.State storage state,
    address from,
    uint256 primaryAmount,
    uint256 secondaryAmount
)
    external
{
    require(isWithdrawValid(state, msg.sender, from, primaryAmount, secondaryAmount), Errors.WITHDRAW_INVALID);
    state.pendingPrimaryWithdraw[msg.sender] = primaryAmount;
    state.pendingSecondaryWithdraw[msg.sender] = secondaryAmount;
    state.withdrawExecutionTimestamp[msg.sender] = block.timestamp + state.withdrawTimeLock;
    emit RequestWithdraw(msg.sender, primaryAmount, secondaryAmount, state.withdrawExecutionTimestamp[msg.sender]);
}

As shown above the withdraw is accidentally queue to msg.sender NOT the from address. This means that all withdraws started on behalf of another user will actually trigger a withdraw from the operator. The result is that withdraw cannot be initiated on behalf of other users, even if the allowance is set properly, leading to irretrievable funds

Impact

Requesting withdraws for other users is broken and strands funds

Code Snippet

Funding.sol#L69-L82

Tool used

Manual Review

Recommendation

Change all occurrences of msg.sender in stage changes to from instead.

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid because { This is valid and a dupp of 082 with minimal impact}

[detectiveking123]

@nevillehuang do you believe this has enough impact to be considered valid?

[JoscelynFarr]

Fixed PR: JOJOexchange/smart-contract-EVM@82b5c85

[IAm0x52]

Fix looks good. Now uses from instead of msg.sender

[nevillehuang]

@detectiveking123 @JoscelynFarr #30 (comment)