Wild Credit contest - WatchPug's results

Decentralized lending protocol with isolated lending pairs.

General Information

Platform: Code4rena

Start Date: 23/09/2021

Pot Size: $50,000 USDC

Total HM: 5

Participants: 14

Period: 7 days

Judge: ghoulsol

Total Solo HM: 3

Id: 32

League: ETH

Wild Credit

Findings Distribution

Researcher Performance

Rank: 1/14

Findings: 4

Award: $19,617.17

🌟 Selected for report: 4

🚀 Solo Findings: 1

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Wild Credit

Findings Information

🌟 Selected for report: 0xRajeev

Also found by: WatchPug

Labels

bug

duplicate

3 (High Risk)

Awards

5947.4124 USDC - $5,947.41

External Links

Link to GitHub issue

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/LendingPair.sol#L660-L684

In _supplyCreditUni(), the calculation of the collateral value of tokenB supply is using _priceB instead of _priceA, which can lead to undercollateralized loans.

  function _supplyCreditUni(
    address _account,
    address _returnToken,
    uint    _priceA,
    uint    _priceB,
    uint    _colFactorA,
    uint    _colFactorB
  ) internal view returns(uint) {

    if (uniPosition[_account] > 0) {

      (uint amountA, uint amountB) = uniV3Helper.positionAmounts(uniPosition[_account], _priceA, _priceB);

      uint supplyA = _convertTokenValues(tokenA, _returnToken, amountA, _priceA, _priceB);
      uint supplyB = _convertTokenValues(tokenB, _returnToken, amountB, _priceB, _priceB);

      uint creditA = supplyA * _colFactorA / 100e18;
      uint creditB = supplyB * _colFactorB / 100e18;

      return (creditA + creditB);

    } else {
      return 0;
    }
  }

Impact

Undercollateralized debts cannot be liquidated and it leads to bad debts to the protocol.

An attacker can deposit a small sum of collateral asset and borrow a rather large amount of asset, essentially steal funds from the protocol.

Proof of Concept

Given:

tokenA is BTC, tokenB is USDC;
Price of BTC is 5000 USDC;
The collateral factor is 80%.

An attacker can:

Deposit a Uni v3 position of 0 BTC and 500 USDC;
The miscalculation made the protocol believes that the attacker has 500 BTC worth of collateral;
Borrow 400 BTC.

The attacker steals ~399.9 BTC from the protocol.

Recommendation

Consider changing to:

uint supplyB = _convertTokenValues(tokenB, _returnToken, amountB, _priceB, _priceA);

#0 - talegift
2021-10-03T04:01:13Z

Duplicate #70

Findings Information

🌟 Selected for report: WatchPug

Labels

bug

3 (High Risk)

disagree-with-severity

sponsor-confirmed

Awards

13216.4719 USDC - $13,216.47

External Links

Link to GitHub issue

Handle

WatchPug

Vulnerability details

When the liquidator is trying to liquidate a undercolldarezed loan by calling liquidateAccount(), it calls _unwrapUniPosition() -> uniV3Helper.removeLiquidity() -> positionManager.decreaseLiquidity().

However, when the Uni v3 position has 0 liquidity, positionManager.decreaseLiquidity() will fail.

See: https://github.com/Uniswap/v3-periphery/blob/main/contracts/NonfungiblePositionManager.sol#L265

Based on this, a malicious user can escaped liquidation by depositing a Uni v3 position with 0 liquidity.

Impact

Undercollateralized debts cannot be liquidated and it leads to bad debts to the protocol.

A malicious user can take advantage of this by creating long positions on the collateral assets and take profit on the way up, and keep taking more debt out of the protocol, while when the price goes down, the debt can not be liquidated and the risks of bad debt are paid by the protocol.

Proof of Concept

A malicious user deposits some collateral assets and borrow the max amount of debt;
The user deposits a Uni v3 position with 0 liquidity;
When the market value of the collateral assets decreases, the liquadation will fail as positionManager.decreaseLiquidity() reverts.

Recommendation

Check if liquidity > 0 when removeLiquidity.

#0 - talegift
2021-10-03T04:39:11Z

Valid issue. Good catch.

Severity should be lowered to 2 as it doesn't allow direct theft of funds and the loss would only occur under specific external conditions.

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements

https://docs.code4rena.com/roles/wardens/judging-criteria#estimating-risk-tl-dr

#1 - ghoul-sol
2021-10-12T04:28:40Z

To my understanding, bad position would affect the whole protocol and a loss would have to be paid by other participans which means funds can be drained. For that reason, I'm keeping high risk.

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Auditors

Browse

Contests

Browse

Get in touch

Contact Twitter

Findings Information

🌟 Selected for report: 0xRajeev

Also found by: WatchPug

Labels

bug

duplicate

G (Gas Optimization)

Awards

52.3013 USDC - $52.30

External Links

Link to GitHub issue

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-09-wildcredit/blob/main/contracts/LendingPair.sol#L125-L136

function uniClaimDeposit() external {
    (uint amountA, uint amountB) = _uniCollectFees(msg.sender);
    _mintSupplyAmount(tokenA, msg.sender, amountA);
    _mintSupplyAmount(tokenB, msg.sender, amountB);
  }

  // claim & withdraw uniswap fees
  function uniClaimWithdraw() external {
    (uint amountA, uint amountB) = _uniCollectFees(msg.sender);
    _safeTransfer(tokenA, msg.sender, amountA);
    _safeTransfer(tokenB, msg.sender, amountB);
  }

amountA and amountB can be 0, Check if amount > 0 before _safeTransfer can save gas.

#0 - talegift
2021-10-02T08:44:38Z

Duplicate #82

Findings Information

🌟 Selected for report: WatchPug

Labels

bug

G (Gas Optimization)

sponsor-confirmed

Awards

116.225 USDC - $116.23

External Links

Link to GitHub issue

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/LendingPair.sol#L338-L339

_supplyBalanceConverted is unnecessary when fromToken and toToken are the same. Change to _supplyOf can save gas.

uint creditA   = _supplyBalanceConverted(_account, tokenA, tokenA, priceA, priceA) * colFactorA / 100e18;
uint creditB   = _supplyBalanceConverted(_account, tokenB, tokenA, priceB, priceA) * colFactorB / 100e18;

Recommendation

Change to:

uint creditA   = _supplyOf(tokenA, _account) * colFactorA / 100e18;
uint creditB   = _supplyBalanceConverted(_account, tokenB, tokenA, priceB, priceA) * colFactorB / 100e18;

Findings Information

🌟 Selected for report: WatchPug

Labels

bug

G (Gas Optimization)

sponsor-acknowledged

Awards

116.225 USDC - $116.23

External Links

Link to GitHub issue

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-09-wildcredit/blob/main/contracts/LendingPair.sol#L93-L96

  function initialize(
    address _lpTokenMaster,
    address _lendingController,
    address _uniV3Helper,
    address _feeRecipient,
    address _tokenA,
    address _tokenB
  ) external {
    require(tokenA == address(0), "LendingPair: already initialized");
    require(_tokenA != address(0) && _tokenB != address(0), "LendingPair: cannot be ZERO address");

    lendingController = ILendingController(_lendingController);
    uniV3Helper       = IUniswapV3Helper(_uniV3Helper);
    feeRecipient      = _feeRecipient;
    tokenA = _tokenA;
    tokenB = _tokenB;
    lastBlockAccrued[tokenA] = block.number;
    lastBlockAccrued[tokenB] = block.number;

    decimals[tokenA] = IERC20(tokenA).decimals();
    decimals[tokenB] = IERC20(tokenB).decimals();

    require(decimals[tokenA] >= 6 && decimals[tokenB] >= 6, "LendingPair: min 6 decimals");

    lpToken[tokenA] = _createLpToken(_lpTokenMaster, tokenA);
    lpToken[tokenB] = _createLpToken(_lpTokenMaster, tokenB);
  }

Cache and check token decimals before write storage can save gas.

Recommendation

Consider changing to:

uint decimalsTokenA = IERC20(tokenA).decimals();
uint decimalsTokenB = IERC20(tokenB).decimals();

require(decimalsTokenA  >= 6 && decimalsTokenB >= 6, "LendingPair: min 6 decimals");

decimals[tokenA] = decimalsTokenA;
decimals[tokenB] = decimalsTokenB;

Findings Information

🌟 Selected for report: cmichel

Also found by: WatchPug

Labels

bug

duplicate

G (Gas Optimization)

Awards

52.3013 USDC - $52.30

External Links

Link to GitHub issue

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/LendingPair.sol#L221-L235

function repayAllETH(address _account, uint _maxAmount) external payable nonReentrant {
    _validateToken(address(WETH));
    accrue(address(WETH));

    uint amount = _repayShares(_account, address(WETH), debtSharesOf[address(WETH)][_account]);
    require(msg.value >= amount, "LendingPair: insufficient ETH deposit");
    require(amount <= _maxAmount, "LendingPair: amount <= _maxAmount");

    _depositWeth();
    uint refundAmount = msg.value > amount ? (msg.value - amount) : 0;

    if (refundAmount > 0) {
      _wethWithdrawTo(msg.sender, refundAmount);
    }
  }

Recommendation

Use Checks-Effects-Interactions pattern for all functions.

#0 - talegift
2021-10-01T13:05:29Z

Dupllcate #49

#1 - ghoul-sol
2021-10-12T01:52:42Z

it's gas optimization

Wild Credit contest - WatchPug's results

Decentralized lending protocol with isolated lending pairs.

General Information

Findings Distribution

Researcher Performance

External Links

[H-01] Miscalculation of `_supplyCreditUni` allows undercollateralized loan - $5,947.41

Findings Information

Labels

Awards

External Links

Handle

Vulnerability details

Impact

Proof of Concept

Recommendation

#0 - talegift2021-10-03T04:01:13Z

🚀 [H-02] Liquidation can be escaped by depositing a Uni v3 position with 0 liquidity - $13,216.47

Findings Information

Labels

Awards

External Links

Handle

Vulnerability details

Impact

Proof of Concept

Recommendation

#0 - talegift2021-10-03T04:39:11Z

#1 - ghoul-sol2021-10-12T04:28:40Z

Gas Report - $453.29

Gas Report - $453.29

[G-04] Check if amount > 0 can save gas - $52.30

Findings Information

Labels

Awards

External Links

Handle

Vulnerability details

#0 - talegift2021-10-02T08:44:38Z

🚀 [G-12] Change unnecessary _borrowBalanceConverted to _debtOf can save gas - $116.23

Findings Information

Labels

Awards

External Links

Handle

Vulnerability details

Recommendation

🚀 [G-13] Change unnecessary _supplyBalanceConverted to _supplyOf can save gas - $116.23

Findings Information

Labels

Awards

External Links

Handle

Vulnerability details

Recommendation

🚀 [G-14] Cache and check decimals before write storage can save gas - $116.23

Findings Information

Labels

Awards

External Links

Handle

Vulnerability details

Recommendation

[G-15] `repayAllETH()` The check if amount is valid can be done earlier - $52.30

Findings Information

Labels

Awards

External Links

Handle

Vulnerability details

Recommendation

#0 - talegift2021-10-01T13:05:29Z

#1 - ghoul-sol2021-10-12T01:52:42Z

#0 - talegift
2021-10-03T04:01:13Z

#0 - talegift
2021-10-03T04:39:11Z

#1 - ghoul-sol
2021-10-12T04:28:40Z

#0 - talegift
2021-10-02T08:44:38Z

#0 - talegift
2021-10-01T13:05:29Z

#1 - ghoul-sol
2021-10-12T01:52:42Z