Lines of code

Vulnerability details

Impact

Malicious user can call the collectFees function with other user’s address function collectFees(address _account, uint256[] memory _ids) who has valid claim and clear the _unclaimedFees. This directly affecting the _unclaimedFees of the other user.
A valid user who has the _unclaimedFees will loose his/her unclaimedFees.

#Proof of Concept

The function takes the _account and array of _ids as input without checking whether the msg.sender is the _account or not.
Whether the _ids has valid length or not, the function do the delete _unclaimedFees[_account]; of the _account and proceeding for further operation.

function collectFees(address _account, uint256[] memory _ids)
    external
    override
    nonReentrant
    returns (uint256 amountX, uint256 amountY)
{
    unchecked {
        bytes32 _unclaimedData = _unclaimedFees[_account];
        delete _unclaimedFees[_account];


        amountX = _unclaimedData.decode(type(uint128).max, 0);
        amountY = _unclaimedData.decode(type(uint128).max, 128);


        for (uint256 i; i < _ids.length; ++i) {
            uint256 _id = _ids[i];
            uint256 _balance = balanceOf(_account, _id);


            if (_balance != 0) {
                Bin memory _bin = _bins[_id];


                (uint256 _amountX, uint256 _amountY) = _getPendingFees(_bin, _account, _id, _balance);
                _updateUserDebts(_bin, _account, _id, _balance);


                amountX += _amountX;
                amountY += _amountY;
            }
        }


        if (amountX != 0) {
            _pairInformation.feesX.total -= uint128(amountX);
        }
        if (amountY != 0) {
            _pairInformation.feesY.total -= uint128(amountY);
        }


        tokenX.safeTransfer(_account, amountX);
        tokenY.safeTransfer(_account, amountY);


        emit FeesCollected(msg.sender, _account, amountX, amountY);
    }
}

When look at the tokenX.safeTransfer(_account, amountX);, the safeTransfer is no checking valid amount. Here also, the function will not revert.
<https://github.com/code-423n4/2022-10-traderjoe/blob/79f25d48b907f9d0379dd803fc2abc9c5f57db93/src/libraries/TokenHelper.sol#L40-L52&gt;

Tools Used

VS code

Recommended Mitigation Steps

Add validation to check whether the msg.sender is the _account or not.

if(msg.sender != _account)
revert(“Invalid caller”);

The text was updated successfully, but these errors were encountered:

👎 1 Shungy reacted with thumbs down emoji

All reactions

• 👎 1 reaction