-
Notifications
You must be signed in to change notification settings - Fork 3
Added 10 reentrancy vulnerability type smart contracts #94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds multiple new Solidity contracts under audit_engine/smart_contracts/reentrancy/*.sol implementing balance withdrawal/transfer flows via low-level calls before state updates, plus one confirm-style call. Each file introduces a contract with public mappings and functions performing external calls and then mutating balances or credits. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant C as Contract (e.g., Bank/EtherStore)
participant E as External Address/Contract
U->>C: withdraw/collect/buyOne(...)
Note over C: Read balance/amount from mapping/args
C->>E: Low-level call with value (call.value(...)(data))
alt call succeeds
Note over C: Post-call state update (e.g., balance -= amt or = 0)
C-->>U: return / success
else call fails
C-->>U: revert/throw/false
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. ✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 15
♻️ Duplicate comments (2)
audit_engine/smart_contracts/reentrancy/40732.sol (2)
6-9
: Same CEI reentrancy issue as 40749.sol.External call precedes balance reset; see suggested fix in audit_engine/smart_contracts/reentrancy/40749.sol Lines 6-9.
1-2
: Missing SPDX/pragma; pin to 0.4.x.Matches 40749.sol; add pragma/license.
🧹 Nitpick comments (4)
audit_engine/smart_contracts/reentrancy/40748.sol (1)
1-4
: Missing pragma/SPDX and explicit visibility.Pin to 0.4.x and declare visibility for clarity.
+pragma solidity 0.4.24; +// SPDX-License-Identifier: UNLICENSED + -contract BancorBuyer { +contract BancorBuyer {audit_engine/smart_contracts/reentrancy/40742.sol (1)
1-4
: Add pragma/SPDX; consider public getter on balances.Pin compiler and add license. If you want external visibility for tests, expose userBalances as public.
+pragma solidity 0.4.24; +// SPDX-License-Identifier: UNLICENSED + -contract SendBalance { +contract SendBalance { @@ - mapping (address => uint) userBalances ; + mapping (address => uint) public userBalances;audit_engine/smart_contracts/reentrancy/40499.sol (1)
1-6
: Add pragma/SPDX and explicit visibility.Pin to 0.4.x; declare mapping visibility if you want external reads.
+pragma solidity 0.4.24; +// SPDX-License-Identifier: UNLICENSED + contract Wallet { @@ - mapping (address => uint) m_txs; + mapping (address => uint) public m_txs;audit_engine/smart_contracts/reentrancy/40747.sol (1)
6-6
: Unused variable ‘putBlock’.Remove it to avoid dead state.
- uint putBlock;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
audit_engine/smart_contracts/reentrancy/40499.sol
(1 hunks)audit_engine/smart_contracts/reentrancy/40732.sol
(1 hunks)audit_engine/smart_contracts/reentrancy/40735.sol
(1 hunks)audit_engine/smart_contracts/reentrancy/40736.sol
(1 hunks)audit_engine/smart_contracts/reentrancy/40742.sol
(1 hunks)audit_engine/smart_contracts/reentrancy/40745.sol
(1 hunks)audit_engine/smart_contracts/reentrancy/40746.sol
(1 hunks)audit_engine/smart_contracts/reentrancy/40747.sol
(1 hunks)audit_engine/smart_contracts/reentrancy/40748.sol
(1 hunks)audit_engine/smart_contracts/reentrancy/40749.sol
(1 hunks)
function confirm(address _h, uint value, byte data) returns (bool) { | ||
if (m_txs[_h] != 0) { | ||
_h.call.value(value)(data); | ||
m_txs[_h] -= value; | ||
return true; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple issues: reentrancy, unchecked call result, and underflow.
- External call before decrement (reentrancy).
- Not checking call success.
- m_txs[_h] can underflow if value > balance.
- byte is a single byte; likely meant bytes.
- function confirm(address _h, uint value, byte data) returns (bool) {
- if (m_txs[_h] != 0) {
- _h.call.value(value)(data);
- m_txs[_h] -= value;
- return true;
- }
- }
+ function confirm(address _h, uint value, bytes data) returns (bool) {
+ uint bal = m_txs[_h];
+ if (bal == 0 || value == 0) { return false; }
+ require(bal >= value);
+ m_txs[_h] = bal - value; // effects first
+ require(_h.call.value(value)(data)); // interaction after
+ return true;
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
function confirm(address _h, uint value, byte data) returns (bool) { | |
if (m_txs[_h] != 0) { | |
_h.call.value(value)(data); | |
m_txs[_h] -= value; | |
return true; | |
} | |
} | |
function confirm(address _h, uint value, bytes data) returns (bool) { | |
uint bal = m_txs[_h]; | |
if (bal == 0 || value == 0) { return false; } | |
require(bal >= value); | |
m_txs[_h] = bal - value; // effects first | |
require(_h.call.value(value)(data)); // interaction after | |
return true; | |
} |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40499.sol around lines 7-13: the
function performs an external call before updating state, doesn't check the call
result, allows underflow by not ensuring value <= m_txs[_h], and uses byte
instead of bytes; fix by first require(value <= m_txs[_h]) to prevent underflow,
subtract m_txs[_h] -= value before any external call (or use the
Checks-Effects-Interactions pattern), change the parameter type from byte to
bytes (or bytes calldata for external functions), perform the external call
using (bool success, ) = _h.call{value: value}(data) and require(success) (or
handle failure), and mark appropriate visibility and return behaviour so the
function explicitly returns a boolean.
|
||
contract dumbDAO { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add SPDX and explicit pragma.
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity 0.4.26;
contract dumbDAO {
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
contract dumbDAO { | |
// SPDX-License-Identifier: UNLICENSED | |
pragma solidity 0.4.26; | |
contract dumbDAO { |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40735.sol around lines 1 to 2, the
file is missing an SPDX license identifier and an explicit pragma solidity
directive; add a SPDX-License-Identifier comment at the top (e.g., a permissive
or project-consistent identifier) and add a pragma solidity statement specifying
the compiler version range (e.g., a caret version like ^0.8.0 or the exact
minimum required by the code) as the first non-comment lines so the compiler and
license scanners can correctly process the contract.
function withdraw(address _recipient) returns (bool) { | ||
|
||
if (_recipient.call.value(balances[msg.sender])()) { | ||
balances[msg.sender] = 0; | ||
return true; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Reentrancy + missing visibility + ambiguous return path.
- External call to arbitrary _recipient before zeroing balance enables reentrancy.
- Visibility not specified (breaks ≥0.5).
- On failure path, function implicitly returns false; be explicit.
- function withdraw(address _recipient) returns (bool) {
-
- if (_recipient.call.value(balances[msg.sender])()) {
- balances[msg.sender] = 0;
- return true;
- }
- }
+ function withdraw(address _recipient) public returns (bool) {
+ uint amount = balances[msg.sender];
+ require(amount > 0);
+ balances[msg.sender] = 0; // effects
+ if (!_recipient.call.value(amount)()) {
+ balances[msg.sender] = amount; // restore on failure
+ return false;
+ }
+ return true;
+ }
If intentionally vulnerable, add a clear header comment stating so instead of fixing logic.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
function withdraw(address _recipient) returns (bool) { | |
if (_recipient.call.value(balances[msg.sender])()) { | |
balances[msg.sender] = 0; | |
return true; | |
} | |
} | |
function withdraw(address _recipient) public returns (bool) { | |
uint amount = balances[msg.sender]; | |
require(amount > 0); | |
balances[msg.sender] = 0; // effects | |
if (!_recipient.call.value(amount)()) { | |
balances[msg.sender] = amount; // restore on failure | |
return false; | |
} | |
return true; | |
} |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40735.sol around lines 6–12, the
withdraw function makes an external call before clearing the caller's balance
(reentrancy risk), lacks visibility, and has an ambiguous return path; fix by
declaring visibility (e.g., public), require the recipient be payable (address
payable _recipient), follow checks-effects-interactions: read
balances[msg.sender] into a local amount, set balances[msg.sender] = 0 before
the external call, perform the external transfer using a safe pattern (or use
call and capture success), on call failure restore the balance and return false
explicitly, and on success return true.
|
||
contract EtherStore { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add SPDX and pin pragma.
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity 0.4.26;
contract EtherStore {
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
contract EtherStore { | |
// SPDX-License-Identifier: UNLICENSED | |
pragma solidity 0.4.26; | |
contract EtherStore { |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40736.sol around lines 1 to 2, the
file is missing an SPDX license identifier and a pinned pragma version; add a
SPDX header (e.g., // SPDX-License-Identifier: MIT) as the first line and add a
specific, pinned pragma solidity statement (e.g., pragma solidity 0.8.19;) on
the next line so the compiler version is explicit and licensing is declared.
function withdrawFunds (uint256 _weiToWithdraw) public { | ||
require(balances[msg.sender] >= _weiToWithdraw); | ||
require(_weiToWithdraw <= withdrawalLimit); | ||
require(msg.sender.call.value(_weiToWithdraw)()); | ||
balances[msg.sender] -= _weiToWithdraw; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Reentrancy in withdrawFunds (CEI violated).
Calls msg.sender before reducing balance. Fix by moving effects before interactions; optionally add a reentrancy guard.
- function withdrawFunds (uint256 _weiToWithdraw) public {
- require(balances[msg.sender] >= _weiToWithdraw);
- require(_weiToWithdraw <= withdrawalLimit);
- require(msg.sender.call.value(_weiToWithdraw)());
- balances[msg.sender] -= _weiToWithdraw;
- }
+ function withdrawFunds (uint256 _weiToWithdraw) public {
+ require(balances[msg.sender] >= _weiToWithdraw);
+ require(_weiToWithdraw <= withdrawalLimit);
+ balances[msg.sender] -= _weiToWithdraw; // effects
+ require(msg.sender.call.value(_weiToWithdraw)()); // interaction
+ }
Optional: add a simple nonReentrant modifier.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
function withdrawFunds (uint256 _weiToWithdraw) public { | |
require(balances[msg.sender] >= _weiToWithdraw); | |
require(_weiToWithdraw <= withdrawalLimit); | |
require(msg.sender.call.value(_weiToWithdraw)()); | |
balances[msg.sender] -= _weiToWithdraw; | |
} | |
function withdrawFunds (uint256 _weiToWithdraw) public { | |
require(balances[msg.sender] >= _weiToWithdraw); | |
require(_weiToWithdraw <= withdrawalLimit); | |
balances[msg.sender] -= _weiToWithdraw; // effects | |
require(msg.sender.call.value(_weiToWithdraw)()); // interaction | |
} |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40736.sol around lines 7–12,
withdrawFunds currently violates the checks-effects-interactions pattern by
calling msg.sender.call.value(...)() before reducing the caller's balance; move
the state change so balances[msg.sender] is decremented (or set to zero) before
any external call, then perform the external transfer and check its return;
additionally, replace raw call usage with a safer pattern (check returned
success explicitly) and/or add a simple nonReentrant modifier (mutex bool guard)
on withdrawFunds to prevent reentrant entry.
|
||
contract PIGGY_BANK { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Pin compiler and add SPDX; code uses pre-0.6 syntax.
Without an explicit pragma these may compile against incompatible solc versions.
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity 0.4.26;
contract PIGGY_BANK {
Run to confirm pragma consistency across the repo:
🏁 Script executed:
#!/bin/bash
rg -n --glob '**/*.sol' 'pragma solidity'
Length of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Checking for the specific reviewed file:"
if [ -f "audit_engine/smart_contracts/reentrancy/40747.sol" ]; then
echo "FOUND: audit_engine/smart_contracts/reentrancy/40747.sol"
sed -n '1,120p' audit_engine/smart_contracts/reentrancy/40747.sol || true
else
echo "Not found: audit_engine/smart_contracts/reentrancy/40747.sol"
fi
echo
echo "Searching repository for .sol files..."
mapfile -t sols < <(find . -type f -name '*.sol' -print)
if [ ${#sols[@]} -eq 0 ]; then
echo "No .sol files found in repo."
exit 0
fi
echo "Found ${#sols[@]} .sol files. Printing first 40 lines of each (filename header included):"
for f in "${sols[@]}"; do
echo "---- FILE: $f ----"
sed -n '1,40p' "$f" || true
done
echo
echo "Searching those files for 'pragma solidity' and 'SPDX-License-Identifier':"
for f in "${sols[@]}"; do
if grep -nE '^\s*pragma\s+solidity|SPDX-License-Identifier' "$f" >/dev/null 2>&1; then
echo "$f matches:"
grep -nE '^\s*pragma\s+solidity|SPDX-License-Identifier' "$f" || true
fi
done
Length of output: 591
Add SPDX header and pin Solidity pragma; fix reentrancy in Collect.
audit_engine/smart_contracts/reentrancy/40747.sol is missing an SPDX header and pragma (it uses pre-0.5 syntax). Add a pinned pragma (e.g. 0.4.26) and SPDX. Also fix the reentrancy: Collect calls msg.sender.call.value(_am)() before updating Accounts[msg.sender] — move the state update before the external call or switch to a withdraw/pull pattern and/or add a reentrancy guard.
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity 0.4.26;
contract PIGGY_BANK {
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40747.sol around lines 1-2, add an
SPDX license header and pin the Solidity pragma to a specific compiler version
(e.g. pragma solidity 0.4.26;) at the top of the file, and fix the reentrancy in
the Collect function by updating Accounts[msg.sender] (set to zero or subtract
_am) before making any external call to msg.sender (or refactor to a
pull/withdraw pattern where users call withdraw to receive funds), and/or add a
simple reentrancy guard (bool locked) to prevent reentrant entries; ensure any
external call uses a safe method (prefer transfer in 0.4.26 or
check-effects-interactions) after state changes.
function Collect(uint _am) public payable { | ||
if(Accounts[msg.sender] >= MinSum && _am <= Accounts[msg.sender]) { | ||
if(msg.sender.call.value(_am)()) { | ||
Accounts[msg.sender] -= _am; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reentrancy via external call before state update (CEI violated).
External call forwards all gas and occurs before debiting balance; attacker can re-enter Collect and drain. If this is not intentionally vulnerable, apply CEI and check the call result.
Apply this diff:
- function Collect(uint _am) public payable {
- if(Accounts[msg.sender] >= MinSum && _am <= Accounts[msg.sender]) {
- if(msg.sender.call.value(_am)()) {
- Accounts[msg.sender] -= _am;
- }
- }
- }
+ function Collect(uint _am) public payable {
+ require(Accounts[msg.sender] >= MinSum && _am <= Accounts[msg.sender]);
+ Accounts[msg.sender] -= _am; // effects
+ require(msg.sender.call.value(_am)()); // interaction
+ }
If this contract is intentionally vulnerable, add a prominent “DO NOT DEPLOY – intentionally vulnerable (reentrancy)” header.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
function Collect(uint _am) public payable { | |
if(Accounts[msg.sender] >= MinSum && _am <= Accounts[msg.sender]) { | |
if(msg.sender.call.value(_am)()) { | |
Accounts[msg.sender] -= _am; | |
} | |
} | |
function Collect(uint _am) public payable { | |
require(Accounts[msg.sender] >= MinSum && _am <= Accounts[msg.sender]); | |
Accounts[msg.sender] -= _am; // effects | |
require(msg.sender.call.value(_am)()); // interaction | |
} |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40747.sol around lines 8 to 13, the
function Collect performs an external call (msg.sender.call.value(_am)()) before
updating Accounts[msg.sender], violating the Checks-Effects-Interactions pattern
and allowing reentrancy; to fix, validate _am against balance first, then
decrement Accounts[msg.sender] (effect) before making the external transfer
(interaction), use a safer transfer pattern (e.g., send/transfer or call with
proper checks and limited gas) and require the call to succeed (revert on
failure); alternatively, if the contract is intentionally vulnerable add a
prominent “DO NOT DEPLOY – intentionally vulnerable (reentrancy)” header at the
top of the file.
function buyOne(address _exchange, uint256 _value, bytes _data) payable public { | ||
require(_exchange.call.value(_value)(_data)); | ||
balances[msg.sender] = balances[msg.sender] - _value; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unchecked subtraction can underflow; also CEI violation enables reentrancy.
If balances[msg.sender] < _value (pre-0.8), Line 8 underflows. Deduct before the call with a bound check; then perform the external call.
- function buyOne(address _exchange, uint256 _value, bytes _data) payable public {
- require(_exchange.call.value(_value)(_data));
- balances[msg.sender] = balances[msg.sender] - _value;
- }
+ function buyOne(address _exchange, uint256 _value, bytes _data) payable public {
+ require(balances[msg.sender] >= _value);
+ balances[msg.sender] -= _value; // effects
+ require(_exchange.call.value(_value)(_data)); // interaction
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
function buyOne(address _exchange, uint256 _value, bytes _data) payable public { | |
require(_exchange.call.value(_value)(_data)); | |
balances[msg.sender] = balances[msg.sender] - _value; | |
} | |
function buyOne(address _exchange, uint256 _value, bytes _data) payable public { | |
require(balances[msg.sender] >= _value); | |
balances[msg.sender] -= _value; // effects | |
require(_exchange.call.value(_value)(_data)); // interaction | |
} |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40748.sol around lines 6 to 9, the
function performs an external call before updating state and subtracts without
checking the balance, which can underflow and enables reentrancy; fix by first
requiring balances[msg.sender] >= _value, decrementing balances[msg.sender] by
_value (the “checks-effects” step), then performing the external call, and
consider using a safe call pattern (e.g. limit gas/handle the returned success)
to avoid reentrancy risks.
|
||
contract Bank{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Missing SPDX and pragma; legacy syntax requires 0.4.x.
Without a pragma these files may compile against 0.8.x and fail. Pin a 0.4.x compiler (throw/call.value syntax compatible) and add license.
+pragma solidity 0.4.24;
+// SPDX-License-Identifier: UNLICENSED
+
contract Bank{
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
contract Bank{ | |
pragma solidity 0.4.24; | |
// SPDX-License-Identifier: UNLICENSED | |
contract Bank{ |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40749.sol around lines 1-2, the file
lacks an SPDX license identifier and a pragma solidity statement which risks
compiling under newer 0.8.x compilers; add a top-of-file SPDX comment (e.g. //
SPDX-License-Identifier: MIT) and pin the compiler to a 0.4.x version compatible
with legacy throw/call.value syntax (e.g. pragma solidity ^0.4.24;) so the
contract compiles with the intended 0.4.x toolchain.
function withdraw(){ | ||
require(msg.sender.call.value(balances[msg.sender])()); | ||
balances[msg.sender] = 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reentrancy: external call before zeroing balance (CEI violation).
Attacker can reenter via fallback before Line 8 and drain funds. Zero-out balance before the call and gate zero withdrawals.
Apply:
- function withdraw(){
- require(msg.sender.call.value(balances[msg.sender])());
- balances[msg.sender] = 0;
- }
+ function withdraw() {
+ uint256 amount = balances[msg.sender];
+ require(amount > 0);
+ balances[msg.sender] = 0; // effects first
+ require(msg.sender.call.value(amount)()); // interaction after
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
function withdraw(){ | |
require(msg.sender.call.value(balances[msg.sender])()); | |
balances[msg.sender] = 0; | |
} | |
function withdraw() { | |
uint256 amount = balances[msg.sender]; | |
require(amount > 0); | |
balances[msg.sender] = 0; // effects first | |
require(msg.sender.call.value(amount)()); // interaction after | |
} |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40749.sol around lines 6 to 9, the
withdraw function performs an external call before zeroing the sender's balance,
allowing reentrancy; change the logic to follow Checks-Effects-Interactions:
first require that balances[msg.sender] > 0, cache the amount to a local
variable, set balances[msg.sender] = 0, then perform the external transfer and
check its return value; alternatively use a reentrancy guard or the pull pattern
(allow owner to withdraw via a withdrawal queue) and avoid using raw
call.value() without proper checks.
Summary by CodeRabbit