Bugvorus - `Comptroller::withdraw` function can be called by non-owner which leads to fund to get withdrawn without the owner intention

[sherlock-admin2]

Bugvorus

Medium

Comptroller::withdraw function can be called by non-owner which leads to fund to get withdrawn without the owner intention

Summary

The withdrawRewards funciton is intended to withdraw rewards when the caller calls this function with the address and token as input. However, it fails to check whether the account given as input is the caller or any arbitary account. Which can potentially leads to protocol to withdraw amount for an address which is not expecting to receive their rewards.

Root Cause

Root cause of this vulnerability is the missing check whether the caller is the same address whose token is being withdrawn

Code-Snippet:

https://github.com/sherlock-audit/2024-06-union-finance-update-2/blob/main/union-v2-contracts/contracts/token/Comptroller.sol#L199

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Malicious caller can calls this function with any arbitary address, leading the protocol to withdraws the rewards of unexpecting user or address without their permission or intention.

PoC

No response

Mitigation

I recommend the following changes:

function withdrawRewards(address account, address token) external override whenNotPaused returns (uint256) {  
        uint256 amount = _accrueRewards(account, token);
        require(account == msg.sender, "Arbitary account");
        if (amount > 0 && unionToken.balanceOf(address(this)) >= amount) {
            users[account][token].accrued = 0;
            unionToken.safeTransfer(account, amount);
            emit LogWithdrawRewards(account, amount);

            return amount;
        } else {
            users[account][token].accrued = amount;
            emit LogWithdrawRewards(account, 0);

            return 0;
        }
    }

[sherlock-admin4]

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

0xmystery commented:

This benefits the account holder without needing to spend gas calling UserManager.withdrawRewards()