Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

ElKu - Curve values are not checked to be in increasing order. Leading to revert of several core functions. #92

Closed
sherlock-admin opened this issue Oct 14, 2022 · 0 comments
Labels

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Oct 14, 2022

ElKu

medium

Curve values are not checked to be in increasing order. Leading to revert of several core functions.

Summary

The curve values in TimeLockPool.sol are not checked to be in increasing order, which might result in all functions which uses the getMultiplier function to revert.

Vulnerability Detail

Looking at the getMultiplier code, the return statement of getMultiplier involves a calculation of curve[n + 1] - curve[n]. This assumes that the curve value at index n+1 is greater than or equal to the one at index n. If that is not the case, the function will revert, resulting in revert of the calling function.

The curve values are set in the following functions:

  1. __TimeLockPool_init
  2. setCurve
  3. setCurvePoint

None of these functions check if the current point on curve is greater than the previous point set.

Impact

There are several functions which use getMultiplier. They are:

  1. deposit in TimeLockPool.sol
  2. extendLock in TimeLockPool.sol
  3. increaseLock in TimeLockPool.sol
  4. fetchOldData in View.sol

Suppose the lock duration passed to getMultiplier falls in the curve between indices i and i+1.
If curve[i+1] < curve[i], then these functions will revert due to an underflow error in subtraction in line 245 of TimeLockPool.

Code Snippet

getMultiplier function:

    function getMultiplier(uint256 _lockDuration) public view returns(uint256) {
        // There is no need to check _lockDuration amount, it is always checked before
        // in the functions that call this function

        // n is the time unit where the lockDuration stands
        uint n = _lockDuration / unit;
        // if last point no need to interpolate
        // trim de curve if it exceedes the maxBonus // TODO check if this is needed
        if (n == curve.length - 1) {
            return 1e18 + curve[n];
        }
        // linear interpolation between points
        return 1e18 + curve[n] + (_lockDuration - n * unit) * (curve[n + 1] - curve[n]) / unit;
    }

Tool used

Manual Review

Recommendation

Include a require condition check before the curve values are set. For example the initialization function where curve values are initially set can be modified like this:

        for (uint i=0; i < _curve.length; i++) {
            if (_curve[i] > _maxBonus) {
                revert MaxBonusError();
            }
						 if (i > 0 && _curve[i] > _curve[i-1]) {
                revert InvalidCurveValue();
            }	
            curve.push(_curve[i]);
        }

Duplicate of #111

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

1 participant