Papr contest - __141345__'s results

Lines of code

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L456-L479 https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L493-L517

Vulnerability details

Impact

If some collateral is used, but later set to not allowed, the already added collaterals can still be used to borrow. If some collateral is disallowed, means the contract will not support the asset anymore, maybe because of security consideration, or market condition changes. However, if continue allowing borrow against the collateral, it will contradict the purpose of disallowing it. And potentially cause fund loss to the contract.

Proof of Concept

Some collateral asset can be added first, and later set to be not allowed by admin function:

File: src/PaprController.sol
365:     function setAllowedCollateral(IPaprController.CollateralAllowedConfig[] calldata collateralConfigs) {

373:             isAllowed[collateralConfigs[i].collateral] = collateralConfigs[i].allowed;

Then this asset can not be added as collateral:

413:     function _addCollateralToVault(address account, IPaprController.Collateral memory collateral) internal {
414:         if (!isAllowed[address(collateral.addr)]) {
415:             revert IPaprController.InvalidCollateral();
416:         }

However, for already added collaterals for this asset, there is no check if it is allowed when _increaseDebt() or _increaseDebtAndSell().

456:     function _increaseDebt(
457:         address account,
458:         ERC721 asset,
459:         address mintTo,
460:         uint256 amount,
461:         ReservoirOracleUnderwriter.OracleInfo memory oracleInfo
462:     ) internal {
463:         uint256 cachedTarget = updateTarget();
464: 
465:         uint256 newDebt = _vaultInfo[account][asset].debt + amount;
466:         uint256 oraclePrice =
467:             underwritePriceForCollateral(asset, ReservoirOracleUnderwriter.PriceKind.LOWER, oracleInfo);
468: 
469:         uint256 max = _maxDebt(_vaultInfo[account][asset].count * oraclePrice, cachedTarget);
470: 
471:         if (newDebt > max) revert IPaprController.ExceedsMaxDebt(newDebt, max);
472: 
473:         if (newDebt >= 1 << 200) revert IPaprController.DebtAmountExceedsUint200();
474: 
475:         _vaultInfo[account][asset].debt = uint200(newDebt);
476:         PaprToken(address(papr)).mint(mintTo, amount);
477: 
478:         emit IncreaseDebt(account, asset, amount);
479:     }

493:     function _increaseDebtAndSell(
494:         address account,
495:         address proceedsTo,
496:         ERC721 collateralAsset,
497:         IPaprController.SwapParams memory params,
498:         ReservoirOracleUnderwriter.OracleInfo memory oracleInfo
499:     ) internal returns (uint256 amountOut) {
500:         bool hasFee = params.swapFeeBips != 0;
501: 
502:         (amountOut,) = UniswapHelpers.swap(
503:             pool,
504:             hasFee ? address(this) : proceedsTo,
505:             !token0IsUnderlying,
506:             params.amount,
507:             params.minOut,
508:             params.sqrtPriceLimitX96,
509:             abi.encode(account, collateralAsset, oracleInfo)
510:         );
511: 
512:         if (hasFee) {
513:             uint256 fee = amountOut * params.swapFeeBips / BIPS_ONE;
514:             underlying.transfer(params.swapFeeTo, fee);
515:             underlying.transfer(proceedsTo, amountOut - fee);
516:         }
517:     }

Tools Used

Manual analysis.

Recommended Mitigation Steps

In function _increaseDebt() and _increaseDebtAndSell(), add the following check:

     function _increaseDebt(
         ,
         ERC721 asset,
         ,
         ,
         
     ) internal {
        if (!isAllowed[address(asset)]) {
            revert IPaprController.InvalidCollateral();
        }

     function _increaseDebtAndSell(
         ,
         ,
         ERC721 collateralAsset,
         ,
         
     ) internal {
        if (!isAllowed[address(collateralAsset)]) {
            revert IPaprController.InvalidCollateral();
        }