20,000 USDC
View results
Submission Details
Severity: medium

Lack of Check on ERC20 Token Transfer in Deposit Function

Summary

The deposit function in the provided Solidity smart contract lacks a proper check on the return value of the transferFrom function, which is used to transfer staking tokens from the user's address to the contract. This omission can potentially lead to a reentrancy attack.

Vulnerability Details

In the deposit function, the contract attempts to transfer staking tokens from the user's address to the contract using the transferFrom function of the ERC20 token. However, the return value of this function, which indicates the success or failure of the transfer, is not checked. Here's the vulnerable code snippet:

function deposit(uint _amount) external {
// Vulnerable code: The return value of transferFrom is unchecked
TKN.transferFrom(msg.sender, address(this), _amount);
balances[msg.sender] += _amount;
updateFor(msg.sender);
}

The vulnerability arises if the transferFrom function fails due to reasons such as the user not having sufficient allowance or balance. If the transfer fails, the balances[msg.sender] will still be updated with the _amount, and the updateFor function will be called, which calculates and updates the user's earned rewards based on the incorrect balance. This leaves the contract in an inconsistent state, leading to potential exploits.

Impact

The lack of a proper check on the return value of transferFrom can open up the possibility of a reentrancy attack. An attacker could potentially exploit this vulnerability to repeatedly call the deposit function before the updateFor function is executed, resulting in incorrect reward calculations and potential loss of funds.

Tools Used

Manual

Recommendations

To address this issue, a proper check on the return value of transferFrom should be added. This can be achieved by using the require statement to verify that the transfer was successful before proceeding with further operations. Here's the updated deposit function with the mitigation:

function deposit(uint _amount) external {
require(TKN.transferFrom(msg.sender, address(this), _amount), "Transfer failed");
balances[msg.sender] += _amount;
updateFor(msg.sender);
}

Support

FAQs

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