Added 10 reentrancy vulnerability type smart contracts #97
Conversation
WalkthroughAdds ten new Solidity contracts under audit_engine/smart_contracts/reentrancy/*.sol, each implementing withdrawal patterns that perform an external call before updating internal balances. New public mappings and functions are introduced across contracts (dumbDAO, EtherStore, Reentrance variants, Private_Bank, SendBalance, SimpleDAO variants, Victim). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant C as Contract (withdraw)
participant R as Recipient/msg.sender
Note over C: Preconditions checked (balance, limits)
User->>C: call withdraw(_amount / _recipient)
rect rgba(230,240,255,0.6)
Note right of C: External call before state update
C->>R: low-level call with value
alt call succeeds
Note over C: Then update internal balance
C-->>User: return success
else call fails
C-->>User: revert/throw or return failure
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
audit_engine/smart_contracts/reentrancy/40761.sol (2)
5-5: Remove unused state.
withdrawnis never read; drop it to avoid confusion.- bool withdrawn = false ;
7-10: Make visibility explicit and annotate intentional vuln.Explicit visibility helps static tools; comment clarifies intent.
- function withdrawBalance(){ + // VULNERABILITY: reentrancy (external call before state update) + function withdrawBalance() public { if (!(msg.sender.call.value(userBalances[msg.sender])())) { throw; } userBalances[msg.sender] = 0; }audit_engine/smart_contracts/reentrancy/40764.sol (1)
6-11: Explicit visibility + intent; note accounting risk.
- Add visibility and intent comment.
- You ignore the
.callreturn; this can burn funds on failed send.- function withdraw(uint amount) { + // VULNERABILITY: reentrancy (external call before state update); ignores call result + function withdraw(uint amount) public { if (credit[msg.sender] >= amount) { msg.sender.call.value(amount)(); credit[msg.sender] -= amount; } }If the goal is only reentrancy (not “burn on failure”), consider reverting on failed send.
audit_engine/smart_contracts/reentrancy/40756.sol (1)
6-12: Explicit visibility + intent comment.Keeps the intended reentrancy while clarifying purpose.
- function CashOut(uint _am) { + // VULNERABILITY: reentrancy (external call before state update) + function CashOut(uint _am) public { if(_am <= balances[msg.sender]) { if(msg.sender.call.value(_am)()) { balances[msg.sender] -= _am; } } }audit_engine/smart_contracts/reentrancy/40753.sol (1)
6-12: Missing explicit visibility and missing return on failure path.Function declares
returns (bool)but can fall through without returning; make it explicit. Add intent comment.- function withdraw(address _recipient) returns (bool) { + // VULNERABILITY: reentrancy (external call before state update) + function withdraw(address _recipient) public returns (bool) { if (balances[msg.sender] == 0){ throw; } if (_recipient.call.value(balances[msg.sender])()) { balances[msg.sender] = 0; return true; } + return false; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
audit_engine/smart_contracts/reentrancy/40753.sol(1 hunks)audit_engine/smart_contracts/reentrancy/40754.sol(1 hunks)audit_engine/smart_contracts/reentrancy/40755.sol(1 hunks)audit_engine/smart_contracts/reentrancy/40756.sol(1 hunks)audit_engine/smart_contracts/reentrancy/40757.sol(1 hunks)audit_engine/smart_contracts/reentrancy/40758.sol(1 hunks)audit_engine/smart_contracts/reentrancy/40761.sol(1 hunks)audit_engine/smart_contracts/reentrancy/40762.sol(1 hunks)audit_engine/smart_contracts/reentrancy/40764.sol(1 hunks)audit_engine/smart_contracts/reentrancy/40765.sol(1 hunks)
🔇 Additional comments (4)
audit_engine/smart_contracts/reentrancy/40762.sol (1)
2-11: Pin compiler + add SPDX header; mark as intentionally vulnerable; quarantine fixtures.rg output: the SPDX/pragma check returned "No files were searched" (inconclusive); duplicate contract names found in audit_engine/smart_contracts/reentrancy — SimpleDAO (3), Reentrance (3), BancorBuyer (2), Bank (2), EtherStore (2), PIGGY_BANK (2), SendBalance (2), TokenBank (2), Victim (2), dumbDAO (2).
Apply:
+// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.4.25; + +/// @dev Intentionally vulnerable to reentrancy for audit_engine tests. DO NOT DEPLOY. contract SimpleDAO {Ensure audit_engine/smart_contracts/** is excluded from production builds/CI and pin the Solidity compiler to 0.4.25 in your hardhat/foundry configs.
audit_engine/smart_contracts/reentrancy/40754.sol (1)
2-13: Pin compiler and add SPDX; optional: mark limit as constant (non-functional).Add a top-of-file SPDX header and an explicit Solidity 0.4.x pragma for reproducible compilation; marking
withdrawalLimitconstantis a non-functional nit if it’s intended to be immutable.+// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.4.25; + +/// @dev Intentionally vulnerable to reentrancy for audit_engine tests. DO NOT DEPLOY. contract EtherStore { - uint256 public withdrawalLimit = 1 ether; + uint256 public withdrawalLimit = 1 ether; // consider: 'constant' if immutable in testsaudit_engine/smart_contracts/reentrancy/40758.sol (1)
2-13: Add SPDX/pragma header and document intentional vulnerabilityAdd license and an explicit Solidity 0.4.x pragma; mark the contract as intentionally vulnerable for audit tests — DO NOT DEPLOY.
+// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.4.25; + +/// @dev Intentionally vulnerable to reentrancy for audit_engine tests. DO NOT DEPLOY. contract Reentrance {audit_engine/smart_contracts/reentrancy/40765.sol (1)
2-13: Add SPDX/pragma and mark contract as intentionally vulnerableFile: audit_engine/smart_contracts/reentrancy/40765.sol — Missing SPDX and pragma; pin to Solidity 0.4.x (e.g. 0.4.25) and add an explicit dev note that this contract is intentionally vulnerable to reentrancy for audit_engine tests (DO NOT DEPLOY).
+// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.4.25; + +/// @dev Intentionally vulnerable to reentrancy for audit_engine tests. DO NOT DEPLOY. contract Victim {Optional (style-only): replace nested
ifwithrequirefor the precondition while leaving the external call before the state update.
|
|
||
| contract dumbDAO { | ||
|
|
There was a problem hiding this comment.
Add SPDX and pragma.
+// SPDX-License-Identifier: MIT
+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: MIT | |
| pragma solidity 0.4.26; | |
| contract dumbDAO { |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40753.sol around lines 1 to 3, the
file is missing the SPDX license identifier and Solidity pragma directive; add
an SPDX comment (e.g. // SPDX-License-Identifier: MIT) as the first line and
then add a suitable pragma solidity line (e.g. pragma solidity ^0.8.0;)
immediately after it to specify the compiler version range supported.
|
|
||
| contract Reentrance { | ||
|
|
There was a problem hiding this comment.
Add SPDX and pragma.
+// SPDX-License-Identifier: MIT
+pragma solidity 0.4.26;
contract Reentrance {📝 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 Reentrance { | |
| // SPDX-License-Identifier: MIT | |
| pragma solidity 0.4.26; | |
| contract Reentrance { |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40755.sol around lines 1 to 3, the
file is missing an SPDX license identifier and a pragma solidity directive; add
a top-of-file SPDX comment (e.g., SPDX-License-Identifier: MIT) on line 1 and a
pragma solidity version statement (e.g., pragma solidity ^0.8.0;) on line 2 to
declare the license and compiler version compatibility before the contract
declaration.
| function withdrawBalance_fixed(){ | ||
| uint amount = userBalance[msg.sender]; | ||
| if(!(msg.sender.call.value(amount)())){ throw; } | ||
| userBalance[msg.sender] = 0; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Function name is misleading; it’s still vulnerable.
Rename to avoid confusion or fix the logic. Given PR goal is “vulnerable samples,” prefer rename + intent comment.
- function withdrawBalance_fixed(){
+ // VULNERABILITY: reentrancy (external call before state update)
+ function withdrawBalance_vulnerable() public {
uint amount = userBalance[msg.sender];
if(!(msg.sender.call.value(amount)())){ throw; }
userBalance[msg.sender] = 0;
}Renaming alters the ABI; confirm no harness depends on the old name.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching repository for function references..."
rg -n --hidden -S "withdrawBalance_fixed|withdrawBalance_vulnerable|withdrawBalance\s*\(" || true
echo
echo "Show file if present: audit_engine/smart_contracts/reentrancy/40755.sol"
if [ -f "audit_engine/smart_contracts/reentrancy/40755.sol" ]; then
sed -n '1,240p' audit_engine/smart_contracts/reentrancy/40755.sol
else
echo "File not found: audit_engine/smart_contracts/reentrancy/40755.sol"
fi
echo
echo "Search for low-level call patterns that indicate reentrancy (msg.sender.call.value / .call.value / call.value):"
rg -n --hidden -S "msg.sender.call.value|call.value\(|\\.call\\.value" || true
echo
echo "Search tests and json/abi files for the original function name (possible harnesses):"
rg -n --hidden -S "withdrawBalance_fixed" -g '!**/node_modules/**' -g '!**/.git/**' || true
rg -n --hidden -S "40755.sol|withdrawBalance_fixed" -g '!**/node_modules/**' -g '!**/.git/**' || trueLength of output: 6361
Function name is misleading; it’s still vulnerable.
Rename to reflect intent (or fix the logic). Repo search found no references to withdrawBalance_fixed, so renaming shouldn't break internal harnesses — confirm any external ABI consumers.
- function withdrawBalance_fixed(){
+ // VULNERABILITY: reentrancy (external call before state update)
+ function withdrawBalance_vulnerable() public {
uint amount = userBalance[msg.sender];
if(!(msg.sender.call.value(amount)())){ throw; }
userBalance[msg.sender] = 0;
}📝 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 withdrawBalance_fixed(){ | |
| uint amount = userBalance[msg.sender]; | |
| if(!(msg.sender.call.value(amount)())){ throw; } | |
| userBalance[msg.sender] = 0; | |
| } | |
| // VULNERABILITY: reentrancy (external call before state update) | |
| function withdrawBalance_vulnerable() public { | |
| uint amount = userBalance[msg.sender]; | |
| if(!(msg.sender.call.value(amount)())){ throw; } | |
| userBalance[msg.sender] = 0; | |
| } |
|
|
||
| contract Private_Bank { | ||
|
|
There was a problem hiding this comment.
Add SPDX and pragma.
+// SPDX-License-Identifier: MIT
+pragma solidity 0.4.26;
contract Private_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 Private_Bank { | |
| // SPDX-License-Identifier: MIT | |
| pragma solidity 0.4.26; | |
| contract Private_Bank { |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40756.sol around lines 1 to 3, the
file is missing the SPDX license identifier and Solidity pragma; add a
SPDX-License-Identifier comment (e.g., "// SPDX-License-Identifier: MIT") on the
very first line and immediately below it add a pragma solidity statement (e.g.,
"pragma solidity ^0.8.0;") to specify the compiler version range supported by
the contract.
| contract Reentrance { | ||
|
|
||
| mapping (address => uint) userBalance; | ||
|
|
||
| function withdrawBalance(){ | ||
| if(!(msg.sender.call.value(userBalance[msg.sender])())){ throw; } | ||
| userBalance[msg.sender] = 0; | ||
| } |
There was a problem hiding this comment.
❓ Verification inconclusive
Legacy patterns: pin compiler; consider explicit visibility.
This uses throw and legacy .call.value(...)(); it must compile on 0.4.x. Add SPDX/pragma and make visibility explicit to reduce warnings while preserving the vulnerable flow.
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity 0.4.25;
+
+/// @dev Intentionally vulnerable to reentrancy for audit_engine tests. DO NOT DEPLOY.
contract Reentrance {
@@
- function withdrawBalance(){
+ function withdrawBalance() public {
if(!(msg.sender.call.value(userBalance[msg.sender])())){ throw; }
userBalance[msg.sender] = 0;
}If you want to avoid the deprecated throw while keeping 0.4.x, you can swap it for revert(); with identical control flow.
Critical: reentrancy — external call before state update in withdrawBalance
File: audit_engine/smart_contracts/reentrancy/40757.sol Lines: 2–9
- Problem: msg.sender.call.value(userBalance[msg.sender])() executes before userBalance[msg.sender] = 0 — allows reentrant calls to drain funds.
- Remediation: move the state update before the external call (checks → effects → interactions) or use a pull-payments/ReentrancyGuard pattern; replace deprecated throw with revert(); add SPDX and an explicit pragma and mark withdrawBalance visibility explicitly.
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40757.sol around lines 2 to 9,
withdrawBalance performs an external call before updating state and uses
deprecated throw and lacks SPDX/pragma/visibility; fix by following
checks→effects→interactions: read the balance into a local uint, set
userBalance[msg.sender] = 0 before making the external transfer (or
alternatively implement a pull-payment pattern or add a ReentrancyGuard),
replace throw with revert() or require-based error handling, and add an SPDX
license identifier, an explicit pragma solidity version, and an explicit
visibility specifier (e.g., public) on withdrawBalance.
|
|
||
| contract SendBalance { | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Add SPDX and pin compiler version for deterministic builds.
Missing license identifier and pragma. Without a pragma these may compile against 0.8.x and fail.
Apply:
+// SPDX-License-Identifier: MIT
+pragma solidity 0.4.26;
contract SendBalance {🏁 Script executed:
#!/bin/bash
# Verify all .sol files in this PR declare a pragma and count deprecated patterns.
fd -e sol audit_engine/smart_contracts/reentrancy \
| while read f; do rg -n '^\s*pragma\s+solidity' "$f" >/dev/null || echo "MISSING_PRAGMA: $f"; done
rg -nH 'throw|\.call\.value\s*\(' audit_engine/smart_contracts/reentrancyLength of output: 5129
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f="audit_engine/smart_contracts/reentrancy/40761.sol"
echo "=== SHOWING $f (first 120 lines) ==="
sed -n '1,120p' "$f" || echo "ERROR: failed to read $f"
echo
echo "=== SEARCH FOR SPDX / PRAGMA IN $f ==="
rg -n --hidden --no-ignore-case "^\s*//\s*SPDX-License-Identifier|^\s*pragma\s+solidity" "$f" || echo "no SPDX/pragma lines found in $f"
echo
echo "=== DIRECTORY-WIDE: report files without a pragma or SPDX ==="
for sol in $(rg -n --files --hidden "audit_engine/smart_contracts/reentrancy" | rg -n "\.sol$" -n | sed -n '1,$p' 2>/dev/null || true); do
# fallback: list all .sol under directory if above fails
:
done
# Robust listing of .sol files and check for pragma/SPDX
find audit_engine/smart_contracts/reentrancy -type f -name '*.sol' -print0 \
| xargs -0 -n1 bash -c 'f="$0"; has=$(rg -n "^\s*pragma\s+solidity|^\s*//\s*SPDX-License-Identifier" "$f" || true); if [ -z "$has" ]; then echo "MISSING_PRAGMA_OR_SPDX: $f"; else echo "OK: $f"; fi'Length of output: 3993
Add SPDX license identifier and pin compiler version for deterministic builds.
audit_engine/smart_contracts/reentrancy/40761.sol (and multiple files in the same directory) are missing an SPDX header and a pragma; the code uses throw and msg.sender.call.value(...)() which target Solidity 0.4.x. Add the following at the top of the file(s):
+// SPDX-License-Identifier: MIT
+pragma solidity 0.4.26;
contract SendBalance {📝 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 SendBalance { | |
| // SPDX-License-Identifier: MIT | |
| pragma solidity 0.4.26; | |
| contract SendBalance { |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40761.sol around lines 1 to 3, the
file is missing an SPDX license identifier and a pinned Solidity compiler
pragma; add an SPDX license header at the very top and immediately below it add
a pragma that pins the compiler to a compatible 0.4.x release (choose a specific
0.4.x version used by the codebase, e.g. 0.4.24) so builds are deterministic;
apply the same change to the other files in the directory that also lack SPDX
and a 0.4.x pragma.
|
|
||
| contract SimpleDAO { | ||
|
|
There was a problem hiding this comment.
Add SPDX and pragma.
+// SPDX-License-Identifier: MIT
+pragma solidity 0.4.26;
contract SimpleDAO {📝 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 SimpleDAO { | |
| // SPDX-License-Identifier: MIT | |
| pragma solidity 0.4.26; | |
| contract SimpleDAO { |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/reentrancy/40764.sol around lines 1 to 3, the
file is missing an SPDX license identifier and a Solidity pragma; add a
top-of-file SPDX line (e.g., "// SPDX-License-Identifier: MIT") followed by a
pragma solidity declaration (e.g., "pragma solidity ^0.8.0;") placed before the
contract declaration to specify the compiler version and license.
Summary by CodeRabbit