stake.link

Summary

The transferRESDL function allows the transfer of locks (ERC721 tokens) between chains.
However, the process does not include deleting the delegation right.
Therefore, when the same token is transferred from chain 1 to chain 2 to chain 1 (the time can be different), the token can be transferred even though the owner has changed.
In other words, the token can be stolen.

Vulnerability Details

Considering when sending from the primary chain to the secondary chain.
In the transferRESDL function, sdlPoolCCIPController.handleOutgoingRESDL is called.

    (address destination, ISDLPool.RESDLToken memory reSDLToken) = sdlPoolCCIPController.handleOutgoingRESDL(
        _destinationChainSelector,
        msg.sender,
        _tokenId
    );

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/ccip/RESDLTokenBridge.sol#L95-L99

In sdlPoolCCIPController.handleOutgoingRESDL, ISDLPoolPrimary(sdlPool).handleOutgoingRESDL is called.

    ISDLPool.RESDLToken memory reSDLToken = ISDLPoolPrimary(sdlPool).handleOutgoingRESDL(
        _sender,
        _tokenId,
        address(this)
    );

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/ccip/SDLPoolCCIPControllerPrimary.sol#L109-L113

At this time, handleOutgoingRESDL has the following processes that directly affect lock.
and the delegation rights to lockId(tokenApprovals[_lockId]) are not removed.

    delete locks[_lockId].amount;
    delete lockOwners[_lockId];
    balances[_sender] -= 1;

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/SDLPoolPrimary.sol#L186-L188

function getApproved(uint256 _lockId) public view returns (address) {
    if (lockOwners[_lockId] == address(0)) revert InvalidLockId();
    return tokenApprovals[_lockId];
}

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/base/SDLPool.sol#L279C1-L283C6

Consider the following flow.

1. attacker gives tokenApprovals[_lockId] to spender1(can be attacker's another address).

2. attacker sends the token from the primary chain to owner1 of the secondary chain.

3. After the time has passed, owner1 sends the token to owner2 in the primary chain.

4. even though owner2 has not authorized anything, spender1 has the delegation right and can steal the token.

This is true even if the secondary and primary chains are reversed.
An attacker can set up a trap by creating a large number of locks and selling them to various people (while keeping the delegation rights).
Even if the user does not intend to attack from the beginning, it is possible that a user may accidentally find out that the previous delegation rights are still in place and steal them.

Impact

The spender can steal tokens even though the current owner has not given permission.

Tools Used

Manual

Recommendations

delete tokenApprovals[_lockId] when moving tokens.