stake.link

Summary

A lock is ERC721 and can be freely traded.
The actual value is affected by the effectiveBalances associated with the lock.
This correspondence is not 1:1; the attacker can contractually promise to give the victim the lock with the higher effectiveBalances and then front-run the transfer transaction to withdraw the money, leaving effectiveBalances=1, so that the transfer is successful, but the value of the lock received is almost worthless.
This is a vulnerability that occurs only in the primary chain.

Vulnerability Details

This can be done for locks in the withdrawable state.
transferFrom can be executed by the owner and can be sent to other users at any time.

function transferFrom(
    address _from,
    address _to,
    uint256 _lockId
) external {
    if (!_isApprovedOrOwner(msg.sender, _lockId)) revert SenderNotAuthorized();
    _transfer(_from, _to, _lockId);
}

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

The internal processing of _transfer differs from the standard ERC721, but there are no particular limitations when sending to users.

function _transfer(
    address _from,
    address _to,
    uint256 _lockId
) internal virtual {
    if (_from != ownerOf(_lockId)) revert TransferFromIncorrectOwner();
    if (_to == address(0)) revert TransferToZeroAddress();
    if (_to == ccipController) revert TransferToCCIPController();
    delete tokenApprovals[_lockId];
    _updateRewards(_from);
    _updateRewards(_to);
    uint256 effectiveBalanceChange = locks[_lockId].amount + locks[_lockId].boostAmount;
    effectiveBalances[_from] -= effectiveBalanceChange;
    effectiveBalances[_to] += effectiveBalanceChange;
    balances[_from] -= 1;
    balances[_to] += 1;
    lockOwners[_lockId] = _to;
    emit Transfer(_from, _to, _lockId);
}

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

withdraw requires that it is not in a lock period.
It can be done for locks whose periods have expired or for locks that were not originally restricted.

function withdraw(uint256 _lockId, uint256 _amount)
    external
    onlyLockOwner(_lockId, msg.sender)
    updateRewards(msg.sender)
{
    if (locks[_lockId].startTime != 0) {
        uint64 expiry = locks[_lockId].expiry;
        if (expiry == 0) revert UnlockNotInitiated();
        if (expiry > block.timestamp) revert TotalDurationNotElapsed();
    }
    uint256 baseAmount = locks[_lockId].amount;
    if (_amount > baseAmount) revert InsufficientBalance();
    emit Withdraw(msg.sender, _lockId, _amount);
    if (_amount == baseAmount) {
        delete locks[_lockId];
        delete lockOwners[_lockId];
        balances[msg.sender] -= 1;
        if (tokenApprovals[_lockId] != address(0)) delete tokenApprovals[_lockId];
        emit Transfer(msg.sender, address(0), _lockId);
    } else {
        locks[_lockId].amount = baseAmount - _amount;
    }
    effectiveBalances[msg.sender] -= _amount;
    totalEffectiveBalance -= _amount;
    sdlToken.safeTransfer(msg.sender, _amount);
}

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

ERC721s are usually traded on the marketplace, but can also be traded privately.
Consider both patterns.

When trading in the Marketplace:

1. the attacker (owner of the lock) lists a lock with high effectiveBalances at a discounted price.

2. the buyer checks the value (effectiveBalances) of the lock and decides to buy it because it is a good deal. The transaction on the market is a simple act of signing for an amount that is usually deducted from one's wallet.

3. the attacker front-runs the transaction and leaving only effectiveBalances=1 by withdrawing (withdrawing the full amount would remove the lock and cannnot transfer)

4. The transfer succeeds, but the buyer receives a nearly worthless lock.

When trading between individuals:
If the buyer pays first, fraud is easy and is not discussed here.

1. the attacker offers to sell a high lock for what appears to be a good deal.

2. the buyer confirms the effectiveBalance of the lock and wants to buy it, but asks for the lock to be sent to him first just in case.

3. The attacker accepts the offer and sends the lock (withdrawing balance it just before the transfer, of course).

4. The buyer confirms receipt of the lock and sends the payment to the attacker.

At this point, if the buyer confirms not only the receipt of the lock but also the effectiveBalance before sending the payment, this fraud can be prevented.
In reality, however, the buyer can only do this if he or she knows about this attack method in advance.
Also, the attacker's cost is the cost of gas(and very low value lock), so he can offer the deal to various people until this is established.
This situation is very bad because it is a breeding ground for fraud.

This is not possible in the secondary chain (withdraw operations are queued and locks with unexecuted operations cannot be transferred).
Another problem is the inconsistency: what is impossible in the secondary chain can be done in the primary chain.

Impact

Attackers succeed in transferring very low-value lock for a high price

Tools Used

Manual

Recommendations

In the primary chain, make a lock in a withdrawable state non-transferable.