Omniscia Parabola Finance Audit

MultiStakingRewards Manual Review Findings

MultiStakingRewards Manual Review Findings

MSR-01M: Potential Circumvention of Lock Time

Description:

The earned rewards are meant to be locked for a predeterminate lockDuration, however, when they are re-staked for a non-zero week the lock is not evaluated. As such, if the lockDuration is greater than the weeks that the user selects in withdrawEarnedOrLock, they are incentivized to circumvent the lock and re-stake them for a shorter period.

Example:

contracts/MultiStakingRewards.sol
393// Earn new tokens
394// Earned tokens receive rewards normally but incur a 50% penalty when
395// withdrawn before lockDuration has passed.
396function earn(address user, uint256 amount) external updateReward(user) {
397    require(earners[msg.sender]);
398    totalSupply = totalSupply.add(amount);
399    Balances storage bal = balances[user];
400    bal.total = bal.total.add(amount);
401    bal.earned = bal.earned.add(amount);
402    uint256 unlockTime = block.timestamp.div(rewardsDuration).mul(rewardsDuration).add(lockDuration);
403    LockedBalance[] storage earnings = userEarnings[user];
404    uint256 idx = earnings.length;
405

406    if (idx == 0 || earnings[idx - 1].unlockTime < unlockTime) {
407        earnings.push(LockedBalance({amount : amount, boostedAmount : 0, unlockTime : unlockTime}));
408    } else {
409        earnings[idx - 1].amount = earnings[idx - 1].amount.add(amount);
410    }
411    emit Staked(user, 0, amount);
412}
413

414// Withdraw earned tokens
415event WithdrawEarnedOrLock(address indexed source, uint256 total, uint256 amount, uint256 penatly);
416

417function withdrawEarnedOrLock(uint256 pid, uint256 week) external nonReentrant updateReward(msg.sender) {
418    require(pid >= 0);
419    require(pid <= userEarnings[msg.sender].length - 1);
420

421    uint256 userEarnedAmount = userEarnings[msg.sender][pid].amount;
422    uint256 userEarnedUnlockTime = userEarnings[msg.sender][pid].unlockTime;
423

424    if (week != 0) {
425        require(weekMultiplier[week] != 0, "Week not supported!");
426

427        Balances storage bal = balances[msg.sender];
428

429        uint boostedAmount = userEarnedAmount.mul(weekMultiplier[week]).div(PRICE_PRECISION);
430        lockedSupply = lockedSupply.add(userEarnedAmount);
431        boostedSupply = boostedSupply.add(boostedAmount);
432        bal.locked = bal.locked.add(userEarnedAmount);
433        bal.boosted = bal.boosted.add(boostedAmount);
434        bal.earned = bal.earned.sub(userEarnedAmount);
435        uint256 unlockTime = block.timestamp.div(rewardsDuration).mul(rewardsDuration).add(rewardsDuration * week);
436        uint256 idx = userLocks[msg.sender][week].length;
437        if (idx == 0 || userLocks[msg.sender][week][idx - 1].unlockTime < unlockTime) {
438            userLocks[msg.sender][week].push(LockedBalance({amount : userEarnedAmount, boostedAmount : boostedAmount, unlockTime : unlockTime}));
439        } else {
440            userLocks[msg.sender][week][idx - 1].amount = userLocks[msg.sender][week][idx - 1].amount.add(userEarnedAmount);
441            userLocks[msg.sender][week][idx - 1].boostedAmount = userLocks[msg.sender][week][idx - 1].boostedAmount.add(boostedAmount);
442        }
443        delete userEarnings[msg.sender][pid];
444

445        emit Staked(msg.sender, week, userEarnedAmount);
446    } else {
447        uint256 penaltyAmount = 0;
448        uint256 remaining = 0;
449        if (userEarnedAmount > 0 && userEarnedUnlockTime > block.timestamp) {
450            penaltyAmount = userEarnedAmount.div(2);
451            remaining = userEarnedAmount.sub(penaltyAmount);
452        } else {
453            remaining = userEarnedAmount;
454        }
455

456        delete userEarnings[msg.sender][pid];
457

458        totalSupply = totalSupply.sub(userEarnedAmount);
459        Balances storage bal = balances[msg.sender];
460        bal.total = bal.total.sub(userEarnedAmount);
461        bal.earned = bal.earned.sub(userEarnedAmount);
462

463        emit WithdrawEarnedOrLock(address(msg.sender), userEarnedAmount, remaining, penaltyAmount);
464

465        stakingToken.safeTransfer(msg.sender, remaining);
466        if (penaltyAmount > 0) {
467            _notifyReward(address(stakingToken), penaltyAmount);
468        }
469    }
470}

Recommendation:

We advise the withdrawEarnedOrLock function to properly evaluated that the new unlockTime is greater than the existing earning unlockTime to mandate proper unlock times for all funds.

Alleviation:

The Parabola team opted not to apply a remediation for this finding in the current iteration of the codebase.

MSR-02M: Inexistent Validation of Proper Withdrawal

Description:

The withdrawExpiredLocks function does not properly validate that a non-zero amount is being unlocked.

Example:

contracts/MultiStakingRewards.sol
608// Withdraw all currently locked tokens where the unlock time has passed
609function withdrawExpiredLocks() external {
610    uint256 amount = 0;
611    uint256 boostedAmount = 0;
612    Balances storage bal = balances[msg.sender];
613    for (uint w = 0; w < supportWeek.length; w++) {
614        LockedBalance[] storage locks = userLocks[msg.sender][supportWeek[w]];
615

616        for (uint i = 0; i < locks.length; i++) {
617            if (locks[i].unlockTime > block.timestamp) {
618                break;
619            }
620            amount = amount.add(locks[i].amount);
621            boostedAmount = boostedAmount.add(locks[i].boostedAmount);
622            delete locks[i];
623        }
624    }
625    bal.locked = bal.locked.sub(amount);
626    bal.boosted = bal.boosted.sub(boostedAmount);
627    bal.total = bal.total.sub(amount);
628    lockedSupply = lockedSupply.sub(amount);
629    boostedSupply = boostedSupply.sub(boostedAmount);
630    totalSupply = totalSupply.sub(amount);
631

632    stakingToken.safeTransfer(msg.sender, amount);
633}

Recommendation:

We advise such validation to be introduced by ensuring that the amount is non-zero after accumulation.

Alleviation:

The function was refactored entirely and the component that withdraws immediately was omitted, thereby we consider this exhibit inapplicable to the latest iteration of the codebase.

MSR-03M: Potentially Incorrect Multiplier Sanitization

Description:

The linked sanitizations ensure that the multiplier is non-zero, however, if it is less than 1000000 it will result in a multiplier less than 1x for stakes.

Example:

contracts/MultiStakingRewards.sol
116function setMaxMultipliers(uint256 _lockedStakeMaxMultiplier) external onlyOwner {
117    require(_lockedStakeMaxMultiplier >= 1, "Multiplier must be greater than or equal to 1");
118    lockedStakeMaxMultiplier = _lockedStakeMaxMultiplier;
119}

Recommendation:

We advise the limit imposed to be equivalent to the 1x multiplier (1000000).

Alleviation:

The Parabola team opted not to apply a remediation for this finding in the current iteration of the codebase.

Prev

LPToken.sol (LPT-M)

Next

ParaToken.sol (PTN-M)

On this page