Jeiwan - Depositors will never get the highest bonus after a new point is added to the end of a curve

[sherlock-admin]

Jeiwan

medium

Depositors will never get the highest bonus after a new point is added to the end of a curve

Summary

When adding new points to a curve, it's required to update the unit state variable. This is done correctly in the setCurve function, but not in the setCurvePoint function. Thus, when a point is added to the end of a curve (such point is supposed to be a point with the highest bonus) the highest bonus will never be applied to deposited amounts because maxLockDuration / uni will never be equal to the new point's index.

Vulnerability Detail

The root cause is that the setCurvePoint function never updates unit after adding a point to the end of the curve:

function setCurvePoint(uint256 _newPoint, uint256 _position) external onlyGov {
    if (_newPoint > maxBonus) {
        revert MaxBonusError();
    }
    if (_position < curve.length) {
        curve[_position] = _newPoint;
    } else if (_position == curve.length) {
        curve.push(_newPoint); // @audit unit is not updated after a new point was added
    } else {
        if (curve.length - 1 < 2) {
            revert ShortCurveError();
        }
        curve.pop();
    }
    emit CurveChanged(_msgSender());
}

Impact

Users will never get the highest bonus if a new point is added to the curve, which undermines the trust in the protocol since the protocol guarantees highest bonus for maximal duration depositors.

Code Snippet

// test/TimeLockPool.ts
it("doesn't update unit after adding a point [audit]", async () => {
    const maxDuration = await timeLockPool.maxLockDuration();

    // The maximal multiplier is 6e18
    expect(await timeLockPool.getMultiplier(maxDuration)).to.eq(constants.WeiPerEther.mul(6));
    // The unit value before a new points is valid.
    expect(await timeLockPool.unit()).to.eq(31536000);

    // Adding a new point with the multiplier 7e18
    const newPoint = (7 * 1e18).toString();
    await timeLockPool.connect(deployer).setCurvePoint(newPoint, 5);

    const addedCurvePoint = await timeLockPool.curve(5);
    expect(addedCurvePoint).to.be.eq(newPoint)

    // The maximal multiplier is still 6e18. The new point is unreachable.
    // Users won't be able to get the highest bonus on their deposits.
    expect(await timeLockPool.getMultiplier(maxDuration)).to.eq(constants.WeiPerEther.mul(6));
    // Unit wasn't changed.
    expect(await timeLockPool.unit()).to.eq(31536000);
})

Tool used

Manual Review

Recommendation

Consider updating unit after added a new point to the end of a curve, similarly to how this is done in the setCurve function.

Duplicate of #101