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

Lack of clearing reSDL approval when transferring to different chain allows stealing of reSDL when it's transferred back to original chain

toshii

Summary

For each reSDL NFT, there is an approval which can be set by calling approve(address _to, uint256 _lockId) to allow another user _to to transfer _lockId on behalf of its owner. However, the logic in the RESDLTokenBridge contract does not clear this approval when the lockId is transferred to another chain. This means that irrespective of who the owner of that lockId is in the future (let's say it was transferred to a new user), if it is ever transferred back to the originating chain, the approved address will be able to perform a transferFrom call to transfer that lockId to themselves, effectively stealing it.

Vulnerability Details

Consider the following attack (here we are assuming that we are starting on the primary chain):

  1. User A owns a reSDL NFT and calls SDLPoolPrimary:approve to approve an address which they also control for this reSDL's lockId.

  2. User A calls RESDLTokenBridge:transferRESDL to transfer their reSDL NFT from chain A to chain B. This triggers SDLPoolCCIPControllerPrimary:handleOutgoingRESDL -> SDLPoolPrimary:handleOutgoingRESDL. Consider what happens in the SDLPoolPrimary:handleOutgoingRESDL call:

function handleOutgoingRESDL(
address _sender,
uint256 _lockId,
address _sdlReceiver
)
...
{
Lock memory lock = locks[_lockId];
​
delete locks[_lockId].amount;
delete lockOwners[_lockId];
balances[_sender] -= 1;
​
uint256 totalAmount = lock.amount + lock.boostAmount;
effectiveBalances[_sender] -= totalAmount;
effectiveBalances[ccipController] += totalAmount;
​
sdlToken.safeTransfer(_sdlReceiver, lock.amount);
​
emit OutgoingRESDL(_sender, _lockId);
​
return lock;
}

Although the ownership data for the _lockId is deleted, the tokenApprovals[_lockId] is never touched, meaning the existing approval will remain for this _lockId.

  1. We assume that User A's CCIP message is successful, and on chain B, user A will now have ownership of the reSDL NFT. On chain B we then assume that user A transfers this reSDL NFT to user B in exchange for funds.

  2. User B decides to transfer this reSDL NFT back to chain A from chain B. This process is very similar to what I described earlier, and ultimately this will trigger SDLPoolPrimary:handleIncomingRESDL on chain A which sets user B as the owner of this reSDL's _lockId on chain A. This function is defined as follows:

function handleIncomingRESDL(
address _receiver,
uint256 _lockId,
Lock calldata _lock
) external onlyCCIPController updateRewards(_receiver) updateRewards(ccipController) {
if (lockOwners[_lockId] != address(0)) revert InvalidLockId();
​
locks[_lockId] = Lock(_lock.amount, _lock.boostAmount, _lock.startTime, _lock.duration, _lock.expiry);
lockOwners[_lockId] = _receiver;
balances[_receiver] += 1;
​
uint256 totalAmount = _lock.amount + _lock.boostAmount;
effectiveBalances[_receiver] += totalAmount;
effectiveBalances[ccipController] -= totalAmount;
​
emit IncomingRESDL(_receiver, _lockId);
}

Again, there is no logic which clears the existing tokenApprovals[_lockId].

  1. Finally, user A, through their secondary address is able to call transferFrom to transfer the _lockId from user B to an address of their choosing, effectively stealing the reSDL.

Impact

A reSDL NFT can be stolen when transferred to another chain & back.

Tools Used

Manual review

Recommendations

The tokenApprovals mapping for a given _lockId should be deleted prior to the reSDL NFT being transferred to another chain in the RESDLTokenBridge contract.

Updates

Lead Judging Commences

0kage Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

stale-approval

Support

FAQs

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