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

SDLPoolSecondary::Redundant storage variable wrongly used

0xAbinash

Summary

In SDLPoolSecondary.sol contract there is a internal storage variable named updateNeeded, the way it was used is equal to not used.

Vulnerability Details

The updateNeeded internal variable is sets to 1 from 0 when new lock is queued i.e when _queueNewLock() is called. Again in _queueLockUpdate() it is checked that if it's value is 0, if yes then assign it to 1. But it has already assigned to 1 during creating new lock. But this operation should have done when a user wants to update a lock position. Additionally while calling handleOutgoingUpdate() this variable's value was not checked whether it is set to 1 or not, although it is set to 0 while calling the handleOutgoingUpdate().

POC

For audit purpose I made the updateNeeded variable public from internal.
Run this test:

it('updateNeededCheck', async () => {
await sdlToken.transferAndCall(
sdlPool.address,
toEther(100),
ethers.utils.defaultAbiCoder.encode(['uint256', 'uint64'], [0, 150 * DAY]) //queued
)
​
await sdlToken
.connect(signers[1])
.transferAndCall(
sdlPool.address,
toEther(200),
ethers.utils.defaultAbiCoder.encode(['uint256', 'uint64'], [0, 150 * DAY])
)
​
await sdlToken
.connect(signers[2])
.transferAndCall(
sdlPool.address,
toEther(100),
ethers.utils.defaultAbiCoder.encode(['uint256', 'uint64'], [0, 150 * DAY])
)
​
await sdlPool.handleOutgoingUpdate()
await sdlPool.handleIncomingUpdate(1)
await sdlPool.executeQueuedOperations([])
console.log('Before calling update for user1', await sdlPool.updateNeeded())
await sdlPool.extendLockDuration(1, 500 * DAY)
console.log('after calling update for user1', await sdlPool.updateNeeded())
await sdlPool.connect(signers[1]).executeQueuedOperations([])
console.log('Before calling update for user2: ', await sdlPool.updateNeeded())
await sdlPool.connect(signers[1]).extendLockDuration(2, 500 * DAY)
console.log('after calling update for user2: ', await sdlPool.updateNeeded())
await sdlPool.connect(signers[2]).executeQueuedOperations([])
console.log('Before calling update for user3: ', await sdlPool.updateNeeded())
await sdlPool.connect(signers[2]).extendLockDuration(3, 500 * DAY)
console.log('after calling update for user3: ', await sdlPool.updateNeeded())
})

The output is:

Before calling update for user1 BigNumber { value: "0" }
after calling update for user1 BigNumber { value: "1" }
Before calling update for user2: BigNumber { value: "1" }
after calling update for user2: BigNumber { value: "1" }
Before calling update for user3: BigNumber { value: "1" }
after calling update for user3: BigNumber { value: "1" }

Impact

Cost unnessary gas & if it used for any operation is future then it will create huge issue.

Tools Used

Manual analysis

Recommendations

To utilize the variable correctly only set it to 1 while queuing an update for a lock in _queuedLockUpdate().

Updates

Lead Judging Commences

0kage Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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