stake.link
stake.link
DeFiHardhatBridge
27,500 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Low Findings

pacelli

[L-01] Duplicate Logic

Summary

The functions SDLPool::staked (line 129) and SDLPool::effectiveBalanceOf (line 138) contain the same logic by reading and returning the same state variable SDLPool::efectiveBalances.

Recommendation

Considering SDLPool inherits from RewardsPoolController, which is an abstract contract and requires the implementation of staked by the child, removeeffectiveBalanceOf from the codebase and use the afforementioned function to read the effective balance of the stakers.

[L-02] Unreachable logic in SDLPoolPrimary::_storeUpdatedLock

Summary

After the lock of a user is updated the _storeUpdatedLock function calculates a diffTotalAmount that represents an amount of effective balance that should be credited or deducted from the owner and from the total effective balance in the pool. This amount it's expected to be either positive or negative, but due to the constraints in the codebase the amount will always be equal or greater with respect to the previous value.

The pool offers to stakers two ways to update their positions:

  • Using SDLPoolPrimary::extendLockDuration function

  • Calling the transferAndCall method on the SDL token

Despite which method the staker chooses to update their position it's expected that the new locking duration MUST be equal or greater than the current period and the user cannot stake zero tokens to the pool.

The following checks are in place in the code base:

  • In SDLPoolPrimary::onTokenTransfer

if (_value == 0) revert InvalidValue();

  • In SDLPool::_updateLock

if ((_lock.expiry == 0 || _lock.expiry > block.timestamp) && _lockingDuration < _lock.duration) {
revert InvalidLockingDuration();
}

This means that when the boost amount and the new stake amount are calculate these amounts will remain the same or increase.

// in `SDLPool::_updateLock`
uint256 baseAmount = _lock.amount + _amount;
uint256 boostAmount = boostController.getBoostAmount(baseAmount, _lockingDuration);

Resulting in diffTotalAmount staying the same or increasing.

int256 diffTotalAmount = int256(lock.amount + lock.boostAmount) -
int256(locks[_lockId].amount + locks[_lockId].boostAmount);

Proof of Concept

The summary can be demonstrated with a series of fuzz tests using with Foundry.

Create a new test file and add the following code.

Test setup
// SPDX-License-Identifier: MIT
pragma solidity >=0.8.0 <0.9.0;
​
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {Test, console2} from "forge-std/Test.sol";
import {ERC677} from "../../../contracts/core/tokens/base/ERC677.sol";
import {StakingAllowance} from "../../../contracts/core/tokens/StakingAllowance.sol";
import {LinearBoostController} from "../../../contracts/core/sdlPool/LinearBoostController.sol";
import {RewardsPool} from "../../../contracts/core/RewardsPool.sol";
import {SDLPool} from "../../../contracts/core/sdlPool/base/SDLPool.sol";
import {SDLPoolPrimary} from "../../../contracts/core/sdlPool/SDLPoolPrimary.sol";
import {ERC721ReceiverMock} from "../../../contracts/core/test/ERC721ReceiverMock.sol";
​
contract SDLPoolPrimaryTest is Test {
StakingAllowance sdlToken;
ERC677 linkToken;
LinearBoostController boostController;
SDLPoolPrimary sdlPool;
SDLPoolPrimary sdlPoolProxy;
RewardsPool rewardsPool;
ERC721ReceiverMock nftReceiver;
​
address CCIP_CONTROLLER = address(this);
Account sdlHolder1 = makeAccount("SDL_HOLDER_1");
​
function setUp() public {
nftReceiver = new ERC721ReceiverMock();
sdlToken = new StakingAllowance("stake.link", "SDL");
linkToken = new ERC677("Chainlink", "LINK", 1_000_000_000 ether);
boostController = new LinearBoostController(4 * 365 days, 4);
sdlPool = new SDLPoolPrimary();
ERC1967Proxy proxy = new ERC1967Proxy(
address(sdlPool),
abi.encodeWithSignature(
"initialize(string,string,address,address)",
"Reward Escrowed SDL",
"reSDL",
address(sdlToken),
address(boostController)
)
);
sdlPoolProxy = SDLPoolPrimary(address(proxy));
rewardsPool = new RewardsPool(address(sdlPoolProxy), address(linkToken));
​
sdlPoolProxy.addToken(address(linkToken), address(rewardsPool));
sdlPoolProxy.setCCIPController(CCIP_CONTROLLER);
​
// mint tokens
sdlToken.mint(CCIP_CONTROLLER, 1_000_000 ether);
sdlToken.mint(sdlHolder1.addr, 10_000 ether);
}
}

Add and execute the following test cases.

Proof A: using `transferAndCall` from the SDL token
function testFuzzAudit__EffectiveBalanceCantDecreaseWithTransfer(uint256 amount) public {
// user stakes and locks
vm.prank(sdlHolder1.addr, sdlHolder1.addr);
sdlToken.transferAndCall(address(sdlPoolProxy), 1_000 ether, abi.encode(0, 365 days));
​
// read effective balances
uint256 holderEffectBalBefore = sdlPoolProxy.effectiveBalanceOf(sdlHolder1.addr);
uint256 totalEffectBalBefore = sdlPoolProxy.totalStaked();
​
// bound input
uint256 holderBalance = sdlToken.balanceOf(sdlHolder1.addr);
amount = bound(amount, 1, holderBalance);
​
// user cannot stake zero tokens
vm.prank(sdlHolder1.addr, sdlHolder1.addr);
vm.expectRevert(SDLPool.InvalidValue.selector);
sdlToken.transferAndCall(address(sdlPoolProxy), 0, abi.encode(1, 365 days));
​
// user cannot update to a smaller locking period
vm.prank(sdlHolder1.addr, sdlHolder1.addr);
vm.expectRevert(SDLPool.InvalidLockingDuration.selector);
sdlToken.transferAndCall(address(sdlPoolProxy), amount, abi.encode(1, 364 days)); // 1 day less
​
// user update his/her position
vm.prank(sdlHolder1.addr, sdlHolder1.addr);
sdlToken.transferAndCall(address(sdlPoolProxy), amount, abi.encode(1, 365 days));
​
// read updated effective balances
uint256 holderEffectBalAfter = sdlPoolProxy.effectiveBalanceOf(sdlHolder1.addr);
uint256 totalEffectBalAfter = sdlPoolProxy.totalStaked();
​
// assertions
assert(holderEffectBalBefore <= holderEffectBalAfter);
assert(totalEffectBalBefore <= totalEffectBalAfter);
}
Proof B: using `extendLockDuration` function
function testFuzzAudit__EffectiveBalanceCantDecreaseWithFunction(uint256 period) public {
vm.prank(sdlHolder1.addr, sdlHolder1.addr);
sdlToken.transferAndCall(address(sdlPoolProxy), 1_000 ether, abi.encode(0, 365 days));
​
// read effective balances
uint256 holderEffectBalBefore = sdlPoolProxy.effectiveBalanceOf(sdlHolder1.addr);
uint256 totalEffectBalBefore = sdlPoolProxy.totalStaked();
​
// user cannot update to a smaller locking period
vm.prank(sdlHolder1.addr, sdlHolder1.addr);
vm.expectRevert(SDLPool.InvalidLockingDuration.selector);
sdlPoolProxy.extendLockDuration(1, 364 days); // 1 day less
​
// bound input
period = bound(period, 365 days, 4 * 365 days);
​
// update lock by extending duration
vm.prank(sdlHolder1.addr, sdlHolder1.addr);
sdlPoolProxy.extendLockDuration(1, uint64(period));
​
// read updated effective balances
uint256 holderEffectBalAfter = sdlPoolProxy.effectiveBalanceOf(sdlHolder1.addr);
uint256 totalEffectBalAfter = sdlPoolProxy.totalStaked();
​
// assertions
assert(holderEffectBalBefore <= holderEffectBalAfter);
assert(totalEffectBalBefore <= totalEffectBalAfter);
}

Recomendation

After calculating diffTotalAmount just add the amount to the effective balance of the owner and the pool.

Updated `_storeUpdatedLock`
function _storeUpdatedLock(
address _owner,
uint256 _lockId,
uint256 _amount,
uint64 _lockingDuration
) internal onlyLockOwner(_lockId, _owner) updateRewards(_owner) {
Lock memory lock = _updateLock(locks[_lockId], _amount, _lockingDuration);
​
int256 diffTotalAmount = int256(lock.amount + lock.boostAmount) -
int256(locks[_lockId].amount + locks[_lockId].boostAmount);
​
- if (diffTotalAmount > 0) {
- effectiveBalances[_owner] += uint256(diffTotalAmount);
- totalEffectiveBalance += uint256(diffTotalAmount);
- } else if (diffTotalAmount < 0) {
- effectiveBalances[_owner] -= uint256(-1 * diffTotalAmount);
- totalEffectiveBalance -= uint256(-1 * diffTotalAmount);
- }
​
+ effectiveBalances[_owner] += uint256(diffTotalAmount);
+ totalEffectiveBalance += uint256(diffTotalAmount);
​
locks[_lockId] = lock;
​
emit UpdateLock(_owner, _lockId, lock.amount, lock.boostAmount, lock.duration);
}
Updates

Lead Judging Commences

0kage Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.