A direct collect interaction does not accrue interests

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/core/Lendgine.sol#L194

Vulnerability details

Impact

A user might miss the interests for his position

Proof of Concept

The protocol has a mechanism to accrue interests for liquidity providers according to the current borrow/lending state. Normally LPs interact with the protocol via the functions exposed in LiquidityManager.sol. When the function LiquidityManager.collect() is called, Lendgine.accruePositionInterest() is triggered first to update the rewardPerPositionStored.

LiquidityManager.sol
229:   /// @notice Collects interest owed to the callers liqudity position
230:   function collect(CollectParams calldata params) external payable returns (uint256 amount) {
231:     ILendgine(params.lendgine).accruePositionInterest();
232:
233:     address recipient = params.recipient == address(0) ? address(this) : params.recipient;
234:
235:     Position memory position = positions[msg.sender][params.lendgine]; // SLOAD
236:
237:     (, uint256 rewardPerPositionPaid,) = ILendgine(params.lendgine).positions(address(this));
238:     position.tokensOwed += FullMath.mulDiv(position.size, rewardPerPositionPaid - position.rewardPerPositionPaid, 1e18);
239:     position.rewardPerPositionPaid = rewardPerPositionPaid;
240:
241:     amount = params.amountRequested > position.tokensOwed ? position.tokensOwed : params.amountRequested;
242:     position.tokensOwed -= amount;
243:
244:     positions[msg.sender][params.lendgine] = position; // SSTORE
245:
246:     uint256 collectAmount = ILendgine(params.lendgine).collect(recipient, amount);
247:     if (collectAmount != amount) revert CollectError(); // extra check for safety
248:
249:     emit Collect(msg.sender, params.lendgine, amount, recipient);
250:   }

On the other hand, Lendgine also exposes a function collect() and it can be called by LPs directly as well. The problem is this function does not call accruePositionInterest() and the reward per position is not updated.

Lendgine.sol
193:   /// @inheritdoc ILendgine
194:   function collect(address to, uint256 collateralRequested) external override nonReentrant returns (uint256 collateral) {
195:     Position.Info storage position = positions[msg.sender]; // SLOAD
196:     uint256 tokensOwed = position.tokensOwed;
197:
198:     collateral = collateralRequested > tokensOwed ? tokensOwed : collateralRequested;
199:
200:     if (collateral > 0) {
201:       position.tokensOwed = tokensOwed - collateral; // SSTORE
202:       SafeTransferLib.safeTransfer(token1, to, collateral);
203:     }
204:
205:     emit Collect(msg.sender, to, collateral);
206:   }

It is understood that the caller can use MultiCall to call accruePositionInterest() and then collect() to prevent this. But there still exists a situation, where a user might miss the interests, and this can be mitigated. Because the LPs might miss interests under some conditions, I submit this as a medium level issue.

Tools Used

Manual Review

Recommended Mitigation Steps

Update the Lendgine.collect() function as below.

function collect(address to, uint256 collateralRequested) external override nonReentrant returns (uint256 collateral) {
+  _accrueInterest();
+  _accruePositionInterest(msg.sender);
  Position.Info storage position = positions[msg.sender]; // SLOAD
  uint256 tokensOwed = position.tokensOwed;

  collateral = collateralRequested > tokensOwed ? tokensOwed : collateralRequested;

  if (collateral > 0) {
    position.tokensOwed = tokensOwed - collateral; // SSTORE
    SafeTransferLib.safeTransfer(token1, to, collateral);
  }

  emit Collect(msg.sender, to, collateral);
}

[c4-judge]

berndartmueller marked the issue as primary issue

[c4-sponsor]

kyscott18 marked the issue as sponsor acknowledged

[kyscott18]

We don't support interacting with the protocol without using the periphery contract.

[c4-judge]

berndartmueller marked the issue as satisfactory

[c4-judge]

berndartmueller marked the issue as selected for report

[berndartmueller]

I'm reconsidering the severity of the finding. Users are expected to use the provided periphery contract. If they don't and interact directly with the core contracts, it's their responsibility to interact with the system correctly in the intended way. As this incorrect interaction with the protocol does not affect other users (other LPs receive even more rewards), I consider QA (Low) to be the appropriate severity.

[c4-judge]

berndartmueller marked the issue as not selected for report

[c4-judge]

berndartmueller changed the severity to QA (Quality Assurance)

[c4-judge]

berndartmueller marked the issue as grade-c