Can't pay fees when paying debt

[code423n4]

Lines of code

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L208

Vulnerability details

Impact

In PaprController#buyAndReduceDebt, the user has the ability to pay debt using the underlying token, delegating to the contract swapping the tokens for Papr tokens.

    function buyAndReduceDebt(address account, ERC721 collateralAsset, IPaprController.SwapParams calldata params)
        external
        override
        returns (uint256)
    {
        bool hasFee = params.swapFeeBips != 0;

        (uint256 amountOut, uint256 amountIn) = UniswapHelpers.swap(
            pool,
            account,
            token0IsUnderlying,
            params.amount,
            params.minOut,
            params.sqrtPriceLimitX96,
            abi.encode(msg.sender)
        );

        if (hasFee) {
            underlying.transfer(params.swapFeeTo, amountIn * params.swapFeeBips / BIPS_ONE);
        }

        _reduceDebt({account: account, asset: collateralAsset, burnFrom: msg.sender, amount: amountOut});

        return amountOut;
    }

Alternatively to increaseDebtAndSell, during the transaction the controller never has any underlying tokens, so in any instance where the fee is set up to be paid, it will fail.

Proof of Concept

Confirm the behavior with the following test, adapted from BuyAndReduceDebt.

function testFee() public {
    vm.startPrank(borrower);

    nft.approve(address(controller), collateralId);
    IPaprController.Collateral[] memory c = new IPaprController.Collateral[](1); 
    c[0] = collateral;
    controller.addCollateral(c);
    IPaprController.SwapParams memory swapParams = IPaprController.SwapParams({
        amount: debt,//*2,
        minOut: 982507,//*2,
        sqrtPriceLimitX96: _maxSqrtPriceLimit({sellingPAPR: true}),
        swapFeeTo: address(0),
        swapFeeBips: 0
    });
    uint256 underlyingOut = controller.increaseDebtAndSell(borrower, collateral.addr, swapParams, oracleInfo);
    IPaprController.VaultInfo memory vaultInfo = controller.vaultInfo(borrower, collateral.addr);
    uint256 fee = 100; //@audit setting up a fee. When fee is zero, the test doesn't fail.
    underlying.approve(address(controller), underlyingOut + underlyingOut * fee / 1e4);
    
    uint160 priceLimit = _maxSqrtPriceLimit({sellingPAPR: false});
    uint256 out = quoter.quoteExactInputSingle({
        tokenIn: address(underlying),
        tokenOut: address(controller.papr()),
        fee: 10000,
        amountIn: underlyingOut * 75 / 100, //@audit paying 75% of debt
        sqrtPriceLimitX96: priceLimit
    });
    swapParams = IPaprController.SwapParams({
        amount: underlyingOut * 75 / 100,
        minOut: out,
        sqrtPriceLimitX96: _maxSqrtPriceLimit({sellingPAPR: false}),
        swapFeeTo: address(5),
        swapFeeBips: fee
    });
    controller.buyAndReduceDebt(borrower, collateral.addr, swapParams);
    //@audit when fee is set up results in "Arithmetic over/underflow"
    vaultInfo = controller.vaultInfo(borrower, collateral.addr);
}

Tools Used

Manual, Foundry

Recommended Mitigation Steps

Use transferFrom if it is expected to pay using the users permission or ensure that the controller has enough underlying tokens to pay the expected amount.

[c4-judge]

trust1995 marked the issue as duplicate of #20

[c4-judge]

trust1995 marked the issue as satisfactory

[C4-Staff]

JeeberC4 marked the issue as duplicate of #196