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

Improper `SDLPrimaryPool` initialization blocks users ability to migrate their `stSDL` tokens

spacelord47

Summary

Improperly implemented "initializer" function in SDLPoolPrimary leaves address delegatorPool state variable uninitialized, which results in all calls toSDLPoolPrimary.migrate revert.

Issue Details

SDLPoolPrimary.migrate function used to migrate SDL tokens from deprecated DelegatorPool to new SDLPoolPrimary:

264: function migrate(
265: address _sender,
266: uint256 _amount,
267: uint64 _lockingDuration
268: ) external {
269: if (msg.sender != delegatorPool) revert SenderNotAuthorized();
270: sdlToken.safeTransferFrom(delegatorPool, address(this), _amount);
271: _storeNewLock(_sender, _amount, _lockingDuration);
272: }

As we can see at line 269, this function is expected to be called only by delegatorPool, which is a state variable in SDLPoolPrimary, and can be update only in the initialize function:

File: SDLPoolPrimary.sol
30: function initialize(
31: string memory _name,
32: string memory _symbol,
33: address _sdlToken,
34: address _boostController
35: ) public reinitializer(2) {
36: if (delegatorPool == address(0)) {
37: __SDLPoolBase_init(_name, _symbol, _sdlToken, _boostController);
38: } else {
39: delegatorPool = ccipController;
40: }
41: }

When SDLPoolPrimary will be deployed and initialize called, delegatorPool will be address(0)(since for the storage, it's a new variable introduced in this update), and the initializer will just call __SDLPoolBase_init, leaving delegatorPool empty.

This basically leaves delegatorPool variable uninitialized and blocks all calls to migrate.

This problem likely introduced due to SDLPoolPrimary.delegatorPool variable previously was in SDLPool.delegatorPool, and now SDLPool.delegatorPool renamed to SDLPool.ccipController, so this renaming resulted to a confusion in the initialize implementation.

Impact

Users will not be able to migrate their stSDL tokens to reSDL as stated in StakeLink FAQ:

Upon release of reSDL, users will be required to migrate stSDL (staked SDL) to reSDL. Users are not required to lock the reSDL, and may immediately withdraw to SDL.

Recommendation

It's not completely clear how exactly the deployment process for this upgrade will looks like (because deploy scripts for new functionality not provided in the repo), but delegatorPool definitely should be set without any conditionals.

My assumption is that SDLPool.ccipController variable (which currently holds previous value for delegatorPool) will be updated after SDLPoolPrimary created(and initialized), so that SDLPoolPrimary can use the old value from ccipController during initialization to set delegatorPool. So a simplified fix might look like this:

diff --git a/contracts/core/sdlPool/SDLPoolPrimary.sol b/contracts/core/sdlPool/SDLPoolPrimary.sol
--- a/contracts/core/sdlPool/SDLPoolPrimary.sol (revision 549b2b8c4a5b841686fceb9c311dca9ac58225df)
+++ b/contracts/core/sdlPool/SDLPoolPrimary.sol (date 1704801938037)
@@ -33,11 +33,8 @@
address _sdlToken,
address _boostController
) public reinitializer(2) {
- if (delegatorPool == address(0)) {
- __SDLPoolBase_init(_name, _symbol, _sdlToken, _boostController);
- } else {
- delegatorPool = ccipController;
- }
+ __SDLPoolBase_init(_name, _symbol, _sdlToken, _boostController);
+ delegatorPool = ccipController;
}
/**
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.