Added 5 delegate call vulnerability type smart contracts #102
Conversation
WalkthroughAdds five new Solidity contracts under audit_engine/smart_contracts/delegate_call/. Each introduces functions that perform low-level delegatecall operations with varying parameters and error handling, including throw/require patterns, self-delegate recursion, and a payable proxy-like withdraw. One function declares a return type without returning a value. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant C1 as multiowned
participant A as sender (address)
Note over C1: Deposit(sender, value)
C->>C1: Deposit(sender, value)
alt sender != 0x0
C1->>A: delegatecall(calldata = value)
Note over C1,A: No return handling
else
Note over C1: No action
end
sequenceDiagram
autonumber
participant U as User
participant S as Safe
participant T as addr (target)
Note over S: requireStackDepth(addr, data)
U->>S: requireStackDepth(addr, data)
alt data == 0x0
S-->>U: throw (revert old-style)
else
S->>T: delegatecall(data)
alt success
S-->>U: return
else
S-->>U: throw (revert)
end
end
sequenceDiagram
autonumber
participant U as User
participant D as StackDepth
Note over D: dig(n)
U->>D: dig(n)
D->>D: delegatecall(selector=0x21835af6, args=n-1)
alt success
D-->>U: return
else
D-->>U: throw (revert)
end
sequenceDiagram
autonumber
participant U as User
participant P as UpgradeabilityProxy
participant L as _logic
Note over P: withdraw(_logic, _data)
U->>P: withdraw(_logic, _data) [payable]
alt _data.length > 0
P->>L: delegatecall(_data)
alt success
P-->>U: return
else
P-->>U: revert(require)
end
else
P-->>U: return (no-op)
end
sequenceDiagram
autonumber
participant E as Extern
participant I as IERC20
participant V as victim
Note over I: sellRewardForWeth(victim, rewardAmount, to)
E->>I: sellRewardForWeth(...)
I->>V: delegatecall(encode("sellRewardForWeth(address,uint256,address)", victim, rewardAmount, to))
Note over I: Declares uint256 return but does not return
I-->>E: (no explicit return value)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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: 6
🧹 Nitpick comments (1)
audit_engine/smart_contracts/delegate_call/dc45.sol (1)
1-6: Naming nit: contract namedIERC20isn’t an interfaceEither rename (e.g.,
ERC20DelegateCaller) or convert to a pure interface (no implementation).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
audit_engine/smart_contracts/delegate_call/dc41.sol(1 hunks)audit_engine/smart_contracts/delegate_call/dc42.sol(1 hunks)audit_engine/smart_contracts/delegate_call/dc43.sol(1 hunks)audit_engine/smart_contracts/delegate_call/dc44.sol(1 hunks)audit_engine/smart_contracts/delegate_call/dc45.sol(1 hunks)
| function Deposit(address sender, uint value) { | ||
| if (sender != 0) { | ||
| sender.delegatecall(value); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
delegatecall(value) is invalid calldata; missing visibility; zero-address check uses 0
This won’t compile on modern Solidity and the calldata is not bytes. Fix to explicit bytes, visibility, and checks.
Apply within this hunk:
- function Deposit(address sender, uint value) {
- if (sender != 0) {
- sender.delegatecall(value);
- }
- }
+ function Deposit(address sender, bytes calldata data) external {
+ if (sender != address(0)) {
+ (bool ok, ) = sender.delegatecall(data);
+ require(ok, "delegatecall failed");
+ }
+ }Add at file top (outside this hunk):
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.25;If you must keep uint value for the dataset semantics, minimally wrap it: (bool ok,) = sender.delegatecall(abi.encode(value)); require(ok);.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for dc41.sol..."
file=$(find . -type f -name 'dc41.sol' -print -quit || true)
if [ -z "$file" ]; then
echo "dc41.sol not found"
exit 0
fi
echo "Found: $file"
echo "---- file path ----"
echo "$file"
echo "---- head (first 200 lines) ----"
sed -n '1,200p' "$file" || true
echo "---- search for SPDX / pragma in file ----"
grep -n -E 'SPDX|pragma' "$file" || true
echo "---- search for delegatecall occurrences in repo (first 100 lines) ----"
grep -RIn --binary-files=without-match -n 'delegatecall' . | sed -n '1,200p' || trueLength of output: 6179
Fix delegatecall usage, add visibility, and add SPDX/pragma
audit_engine/smart_contracts/delegate_call/dc41.sol (lines 3–7): won't compile on modern Solidity and is unsafe — apply the patch below.
- function Deposit(address sender, uint value) {
- if (sender != 0) {
- sender.delegatecall(value);
- }
- }
+ function Deposit(address sender, bytes calldata data) external {
+ if (sender != address(0)) {
+ (bool ok, ) = sender.delegatecall(data);
+ require(ok, "delegatecall failed");
+ }
+ }Add at file top (outside this hunk):
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.25;If you must keep uint value for dataset semantics, minimally wrap it: (bool ok, ) = sender.delegatecall(abi.encode(value)); require(ok);.
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/delegate_call/dc41.sol around lines 3–7, the
function uses an unsafe/old delegatecall form and lacks SPDX/pragma and
visibility; add the following fixes: add file header lines "//
SPDX-License-Identifier: UNLICENSED" and "pragma solidity ^0.8.25;" at the top
of the file (outside this hunk), change the function signature to include
explicit visibility (e.g., "function Deposit(address sender, uint value)
public"), and replace the raw delegatecall with the safe ABI-encoded pattern:
perform the call as "(bool ok, ) = sender.delegatecall(abi.encode(value));
require(ok);" so the call is encoded and its success is checked.
| function requireStackDepth(address addr, bytes32 data) { | ||
| if (data=='') { throw; } | ||
| if (!addr.delegatecall(data)){ throw; } | ||
| } |
There was a problem hiding this comment.
bytes32 vs empty string check, legacy throw, and delegatecall signature
data == '' on bytes32 is invalid; use bytes and length. Update error handling and low-level call.
Apply within this hunk:
- function requireStackDepth(address addr, bytes32 data) {
- if (data=='') { throw; }
- if (!addr.delegatecall(data)){ throw; }
- }
+ function requireStackDepth(address addr, bytes calldata data) external {
+ require(data.length != 0, "empty data");
+ (bool ok, ) = addr.delegatecall(data);
+ require(ok, "delegatecall failed");
+ }Add at file top (outside this hunk):
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.25;| function dig(uint n) public { | ||
| if (!address(this).delegatecall(0x21835af6, n - 1)) throw; | ||
| } |
There was a problem hiding this comment.
Legacy throw/delegatecall + missing base case cause compile errors and potential unbounded recursion
Modern compilers reject throw and the legacy delegatecall(0x..., n-1) form; also n-1 underflows at 0, changing behavior across compiler versions.
Apply within this hunk:
- function dig(uint n) public {
- if (!address(this).delegatecall(0x21835af6, n - 1)) throw;
- }
+ function dig(uint256 n) public {
+ require(n > 0, "n underflow");
+ (bool ok, ) = address(this).delegatecall(
+ abi.encodeWithSelector(this.dig.selector, n - 1)
+ );
+ require(ok, "delegatecall failed");
+ }Add at file top (outside this hunk):
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.25;🤖 Prompt for AI Agents
In audit_engine/smart_contracts/delegate_call/dc43.sol around lines 3-5, the
function uses legacy throw and the old delegatecall ABI with an unsafe n-1 that
underflows; add SPDX and a pragma at the top of the file (outside this hunk) as
instructed, then change dig to first handle a base case (if n == 0 return) to
prevent unbounded recursion/underflow, compute n-1 only after ensuring n>0, and
replace the legacy delegatecall/throw with the modern pattern: perform (bool
success, bytes memory) =
address(this).delegatecall(abi.encodeWithSelector(bytes4(0x21835af6), nm1)) and
require(success) (or revert with an error) instead of throw.
| function withdraw(address _logic, bytes memory _data) public payable { | ||
| if(_data.length > 0) { | ||
| bool success = _logic.delegatecall(_data); | ||
| require(success); | ||
| } |
There was a problem hiding this comment.
Unguarded arbitrary delegatecall is a critical footgun
Anyone can execute arbitrary logic in this contract’s context (storage/balance). If this isn’t intentionally vulnerable test code, restrict caller (e.g., onlyOwner) and/or whitelist _logic.
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/delegate_call/dc44.sol around lines 3 to 7, the
function withdraw performs an unguarded delegatecall allowing anyone to execute
arbitrary logic in the contract’s context; restrict access by adding an
ownership check (e.g., onlyOwner modifier or require(msg.sender == owner))
and/or implement a whitelist of allowed `_logic` addresses (mapping(address =>
bool) approvedLogic and require(approvedLogic[_logic])). Also propagate a clear
revert reason on failure (require(success, "delegatecall failed")) and consider
emitting an event for successful/failed withdraw attempts for auditing.
Tuple return from delegatecall not handled; add minimal safety check
On modern Solidity, delegatecall returns (bool, bytes). Also guard zero logic address.
Apply within this hunk:
function withdraw(address _logic, bytes memory _data) public payable {
if(_data.length > 0) {
- bool success = _logic.delegatecall(_data);
- require(success);
+ require(_logic != address(0), "logic=0");
+ (bool success, bytes memory /*ret*/ ) = _logic.delegatecall(_data);
+ require(success, "delegatecall failed");
}
}📝 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 _logic, bytes memory _data) public payable { | |
| if(_data.length > 0) { | |
| bool success = _logic.delegatecall(_data); | |
| require(success); | |
| } | |
| function withdraw(address _logic, bytes memory _data) public payable { | |
| if(_data.length > 0) { | |
| require(_logic != address(0), "logic=0"); | |
| (bool success, bytes memory /*ret*/ ) = _logic.delegatecall(_data); | |
| require(success, "delegatecall failed"); | |
| } |
🤖 Prompt for AI Agents
In audit_engine/smart_contracts/delegate_call/dc44.sol around lines 3 to 7, the
delegatecall return tuple and a zero-address guard are missing; update the
function to first require(_logic != address(0)) and then perform (bool success,
bytes memory returndata) = _logic.delegatecall(_data), followed by
require(success, "delegatecall failed"); optionally propagate or inspect
returndata as needed for richer error messages.
| function sellRewardForWeth(address victim, uint256 rewardAmount, address to) external returns(uint256) { | ||
| victim.delegatecall(abi.encodeWithSignature("sellRewardForWeth(address,uint256,address)", victim, rewardAmount, to)); | ||
| } |
There was a problem hiding this comment.
Declared return value not returned; delegatecall result ignored
This fails to compile and swallows failures. Return or remove the return type.
Apply within this hunk:
- function sellRewardForWeth(address victim, uint256 rewardAmount, address to) external returns(uint256) {
- victim.delegatecall(abi.encodeWithSignature("sellRewardForWeth(address,uint256,address)", victim, rewardAmount, to));
- }
+ function sellRewardForWeth(address victim, uint256 rewardAmount, address to) external returns (uint256) {
+ (bool ok, bytes memory ret) =
+ victim.delegatecall(abi.encodeWithSignature(
+ "sellRewardForWeth(address,uint256,address)", victim, rewardAmount, to));
+ require(ok, "delegatecall failed");
+ if (ret.length >= 32) {
+ return abi.decode(ret, (uint256));
+ }
+ return 0;
+ }Add at file top (outside this hunk):
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.25;🤖 Prompt for AI Agents
In audit_engine/smart_contracts/delegate_call/dc45.sol around lines 3 to 5, the
function declares a uint256 return but ignores delegatecall result and doesn't
return anything; capture the (bool success, bytes memory data) result of
victim.delegatecall, revert or bubble the error when success is false, decode
the returned bytes to uint256 and return it (e.g., abi.decode(data, (uint256))).
Also add the missing file header at the top (outside this hunk): an SPDX
identifier and pragma solidity ^0.8.25; to satisfy compiler requirements.
Summary by CodeRabbit