search

code-423n4 / 2023-08-pooltogether-findings

4 stars 3 forks source link

Lack of check in `LiquidationPair.sol#_computePeriod()` can lead to DOS #170

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-cgda-liquidator/blob/7f95bcacd4a566c2becb98d55c1886cadbaa8897/src/LiquidationPair.sol#L377-L383

Vulnerability details

Impact

_computePeriod() will revert because lack of check input validation

Proof of Concept

In LiquidationPair.sol, _computePeriod() is used to computes the current auction period: see here. It is called in functions like getPeriodStart() and _checkUpdateAuction().

377:  function _computePeriod() internal view returns (uint256) {
378:    uint256 _timestamp = block.timestamp;
379:    if (_timestamp < periodOffset) {
380:      return 0;
381:    }
382:    return (_timestamp - periodOffset) / periodLength;
383:  }

In L382, _computePeriod() will revert if periodLength == 0 lead to Dos

Tools Used

Manual review

Recommended Mitigation Steps

  1. Add check if (periodLength == 0) revert()
  2. In L379 change from (_timestamp < periodOffset) to (_timestamp <= periodOffset)

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

raymondfam commented 1 year ago

periodLength is an immutable variable assigned at the constructor. The contract would have been unusable with zero assigned to it.

c4-judge commented 1 year ago

HickupHH3 marked the issue as unsatisfactory: Invalid