stake.link

Summary

The SDLPool._transfer function violates the check-effects-interactions pattern by making external calls to _updateRewards before updating critical state variables. This vulnerability could allow an attacker to re-enter the contract and perform actions with unintended consequences.

Finding: The SDLPool._transfer function is vulnerable to a reentrancy attack due to state updates occurring after external calls, which can be exploited by an attacker to manipulate contract state.

Vulnerability Details

The function _transfer makes external calls to _updateRewards for both the sender (_from) and receiver (_to) before updating the effectiveBalances, balances, and lockOwners state variables. A malicious contract could exploit this by re-entering the SDLPool contract during the _updateRewards call and interacting with it in a way that could lead to loss of funds or corrupted contract state.

Impact

If exploited, an attacker could potentially manipulate the contract's state, leading to incorrect balance tracking, unauthorized token transfers, or other unintended effects that could compromise the integrity of the contract and result in financial loss for users.

Proof of Concept (PoC):

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.15;

interface ISDLPool {
function transferFrom(address _from, address _to, uint256 _lockId) external;
}

contract MaliciousContract {
ISDLPool public sdlPool;
address public attacker;
uint256 public lockId;

constructor(address _sdlPool, uint256 _lockId) {
    sdlPool = ISDLPool(_sdlPool);
    attacker = msg.sender;
    lockId = _lockId;
}

// Fallback function used for reentrancy
fallback() external payable {
    sdlPool.transferFrom(attacker, address(this), lockId);
}

function attack() external {
    sdlPool.transferFrom(attacker, address(this), lockId);
}

}
To execute the attack, the attacker deploys MaliciousContract with the SDLPool contract address and a lockId they own, then calls the attack function.

Tools Used

Manual Code Review

Recommendations

Update the _transfer function to follow the check-effects-interactions pattern: perform all state updates before making external calls.
Implement a reentrancy guard to prevent reentrant calls.
Review all contract functions for similar patterns and apply the recommended changes.
Conduct thorough testing and auditing to ensure the security of the contract.