Submission Details
Severity:
Reentrancy attack in `TokenDivider::transferErcTokens` allows entrant to transfer available token balance
**DESCRIPTION**
The `TokenDivider::transferErcTokens` function does not follow CEI (Checks, Effects, Interactions) and as a result, enables participants to transfer available token balance.
**IMPACT**
All token available in the contract can be stolen by a participant
**Proof of Concept:**
1. User enters the contract
2. Attacker sets up a contract with a `fallback` function that calls `TokenDivider::transferErcTokens`
3. Attacker enters the raffle
4. Attacker calls `TokenDivider::transferErcTokens` from their attack contract, it could potentially re-enter your function and manipulate state variables again based on updated balances without proper checks.
details>
<summary>PoC Code</summary>
Below is a simplified example of an attacker contract that exploits the vulnerability:
```js
contract AttackERC20 {
mapping(address => uint256) public balances;
address public target;
constructor(address _target) {
target = _target;
}
function approve(address spender, uint256 amount) external {
// Simulate approving transfer (normally this would update allowance)
balances[msg.sender] += amount;
}
function transferFrom(address sender, address recipient, uint256 amount) external returns (bool) {
require(balances[sender] >= amount);
// Re-entrancy happens here
AttackERC20(target).transferFrom(sender, recipient, amount);
return true;
}
}
```
**Recommendation**
To mitigate reentrancy attacks, consider following these best practices:
1. Use Checks-Effects-Interactions Pattern (CEI); First check all conditions, Then modify all state variables (i.e., perform effects). Finally interact with external contracts.
2. Reentrancy Guard: Use a mutex lock mechanism by implementing a reentrancy guard pattern using modifiers such as OpenZeppelin's ReentrancyGuard.
3. Review External Calls Carefully: Be cautious about interacting with untrusted contracts—make sure they do not have malicious code that could exploit your application logic.
Here’s how you might restructure `TokenDivider::transferErcTokens` based on best practices:
```diff
function transferErcTokens(address nftAddress,address to,uint256 amount) external {
// ... validations
// Check all conditions before making state changes
require(balances[msg.sender][tokenInfo.erc20Address] >= amount, "Not enough balance");
// Effect: Update internal state first before interacting with external contracts
balances[msg.sender][tokenInfo.erc20Address] -= amount;
balances[to][tokenInfo.erc20Address] += amount;
emit TokensTransfered(amount, tokenInfo.erc20Address);
// Interaction: Now safely interact with the ERC20 token
IERC20(tokenInfo.erc20Address).transferFrom(msg.sender,to,amount);
}
```
Rewards breakdown
nSLOC
181This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.