Unauthorized Token Transfer in transferAndJoin

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-07-loopfi/blob/57871f64bdea450c1f04c9a53dc1a78223719164/src/proxy/PoolAction.sol#L75

Vulnerability details

Vulnerability Details

The transferAndJoin function allows any caller to specify any address as the from parameter, potentially enabling unauthorized transfers of tokens from users who have approved the contract.

Impact

1. Unauthorized Token Transfers: An attacker could transfer tokens from any user who has approved the contract, without their consent.
2. Potential for Large-scale Token Theft: If multiple users have approved the contract, an attacker could drain their accounts in a single transaction.

Vulnerable Code

function transferAndJoin(
        address from,
        PermitParams[] calldata permitParams,
        PoolActionParams calldata poolActionParams
    ) external {
        // No check that msg.sender is authorized to transfer from 'from'
        if (from != address(this)) {
            if (poolActionParams.protocol == Protocol.BALANCER) {
                (, address[] memory assets, , uint256[] memory maxAmountsIn) = abi.decode(
                    poolActionParams.args,
                    (bytes32, address[], uint256[], uint256[])
                );

                if (assets.length != permitParams.length) {
                    revert PoolAction__transferAndJoin_invalidPermitParams();
                }

                for (uint256 i = 0; i < assets.length; ) {
                    if (maxAmountsIn[i] != 0) {
                        _transferFrom(assets[i], from, address(this), maxAmountsIn[i], permitParams[i]);
                    }

                    unchecked {
                        ++i;
                    }
                }
            } else if(poolActionParams.protocol == Protocol.PENDLE) {
                (, , TokenInput memory input,) = abi.decode(poolActionParams.args, (address, ApproxParams, TokenInput , LimitOrderData));
                
                if (input.tokenIn != address(0)) {
                    _transferFrom(input.tokenIn, from, address(this), input.netTokenIn, permitParams[0]);
                }
            } else  revert PoolAction__transferAndJoin_unsupportedProtocol();
        }

        join(poolActionParams);
    }
}

Exploit Scenario

1. Alice approves the PoolAction contract to spend her tokens.
2. Bob, an attacker, calls transferAndJoin with Alice's address as the from parameter.
3. The function transfers Alice's tokens to the contract and joins a pool, effectively stealing Alice's tokens.

Recommended Fix

Implement proper access control to ensure that only authorized parties can initiate transfers:

function transferAndJoin(
    address from,
    PermitParams[] calldata permitParams,
    PoolActionParams calldata poolActionParams
) external {
    require(msg.sender == from || isAuthorized(msg.sender, from), "Unauthorized");
    // ... rest of the function ...
}

isAuthorized is a function that checks if the caller is authorized to act on behalf of the from address (e.g., approved operator, owner, etc.).

Assessed type

Access Control