stake.link

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

Unsafe conversion from unsigned to signed integer in initiateUnlock(...) function

Summary

In Solidity explicit casting from unsigned to signed integer are not protected from overflow that's why you need OZ's safeCast library for casting.

Vulnerability Details

In the function below a uint256 boostAmount variable was casted to int256.
First of all explicit casting in solidity is not protected from overflow.
This is casting from uint256 to int256 but in order to use small values I will explain this overflow issue with casting uint8 to int8.

So let's look at the values available in both:

uint8 possible values ranges from 0 to 255 while
int8 possible values ranges from -128 to 127

From the above you can see that it is impossible for int8 to have a value of 129. This means that trying to cast a uint value that is 129 will overflow and result to -127.
Remember the range of values of int8 are -128, -127, -126,.....0.......125, 126, 127 that's why casting 129 will overflow to -127. The same way 128 will overflow to return -128 (note the - sign).

This also means that int8(type(uint8).max) != type(int8).max

Following the same logic above casting a uint256 value boostAmount to a int256 is not safe and would overflow for some range of values since they both have the same range of possible values.

function initiateUnlock(uint256 _lockId) external onlyLockOwner(_lockId, msg.sender) updateRewards(msg.sender) {
Lock memory lock = _getQueuedLockState(_lockId);
if (lock.expiry != 0) revert UnlockAlreadyInitiated();
uint64 halfDuration = lock.duration / 2;
if (lock.startTime + halfDuration > block.timestamp) revert HalfDurationNotElapsed();
uint64 expiry = uint64(block.timestamp) + halfDuration;
lock.expiry = expiry;
uint256 boostAmount = lock.boostAmount;
lock.boostAmount = 0;
effectiveBalances[msg.sender] -= boostAmount;
totalEffectiveBalance -= boostAmount;
queuedLockUpdates[_lockId].push(LockUpdate(updateBatchIndex, lock));
queuedRESDLSupplyChange -= int256(boostAmount);
if (updateNeeded == 0) updateNeeded = 1;
emit QueueInitiateUnlock(msg.sender, _lockId, expiry);
}
Same casting from uint256 to int256 is done in the _queueNewLock(...) and _queueLockUpdate(...) function
- https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/SDLPoolSecondary.sol#L373
- https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/SDLPoolSecondary.sol#L438C13-L439C52

Coded POC

//Foundry Test

function test_unsafecasting() public {
uint256 testAmount = type(uint256).max;

//This test will fail because the value of casting the above will overflow
assertEq(testAmount, int(testAmount);

}
The test above should fail because after casting testAmount to int, it will overflow and no more equal to testAmount.

Impact

overflow leading to incorrect accounting issues.

Tools Used

Foundry

Recommendations

Use openzeppelin's safeCast library for every casting in the codebase.

Updates

Lead Judging Commences

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

Support

FAQs

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