Lender#lend for APWine doesn't validate that pool is swapping same underlying as market underlying

Summary

Lender#Lend for APWine allows the user to specify the address of the pool to used to swap the underlying to AWP principle. A malicious user could abuse this swap using an unexpected pool and extract underlying from the Lender contract.

Vulnerability Detail

function lend(
    uint8 p,
    address u,
    uint256 m,
    uint256 a,
    uint256 r,
    uint256 d,
    address x,
    address pool
) external unpaused(u, m, p) returns (uint256) {
    address principal = IMarketPlace(marketPlace).token(u, m, p);

    // Transfer funds from user to Illuminate
    Safe.transferFrom(IERC20(u), msg.sender, address(this), a);

    uint256 lent;
    {
        // Add the accumulated fees to the total
        uint256 fee = a / feenominator;
        fees[u] = fees[u] + fee;

        // Calculate amount to be lent out
        lent = a - fee;
    }

    // Get the starting APWine token balance
    uint256 starting = IERC20(principal).balanceOf(address(this));

    // Swap on the APWine Pool using the provided market and params
    IAPWineRouter(x).swapExactAmountIn(
        pool,
        apwinePairPath(),
        apwineTokenPath(),
        lent,
        r,
        address(this),
        d,
        address(0)
    );

    // Calculate the amount of APWine principal tokens received after the swap
    uint256 received = IERC20(principal).balanceOf(address(this)) -
        starting;

    // Mint Illuminate zero coupons
    IERC5095(principalToken(u, m)).authMint(msg.sender, received);

    emit Lend(p, u, m, received, a, msg.sender);
    return received;
}

Lender#Lend for APWine allows the user to specify the address of the pool to be used but never checks that the pool being used is appropriate for the specified underlying. A user is unable to specify an outright malicious pool when swapping because that would require the Admin to approve the underlying for the malicious pool, requiring the admin to be phished which is out of scope. What it doesn't protect from is approved pool spoofing. Lender is meant to handle multiple different underlying tokens (i.e. USDC, DAI, USDT, etc.). With pool spoofing the attacker can call the function with one underlying but with a pool that swaps a different underlying token. For example a user could call lend with u = DAI but use the pool for USDC. DAI is an 18 decimal token while USDC is a 6 decimal token, which means the user would pay a very small amount of DAI to swap a significant amount of USDC. To extract value the attacker would sandwich attack the APW pool to steal the traded underlying.

Impact

Attacker can steal all underlying present in the Lender contract

Code Snippet

Lender.sol#L572-L621

Tool used

Manual Review

Recommendation

The user should not be allowed to specify which pool to use. The pool for each principal token should be assigned via a mapping. Instead of the user supplying the pool the mapping should be used to pull the correct pool for each principal token

Duplicate of https://github.com/sherlock-audit/2022-10-illuminate-judging/issues/47

sherlock-audit / 2022-10-illuminate-judging

0x52 - Lender#lend for APWine doesn't validate that pool is swapping same underlying as market underlying #161