Skip to content

Commit 0427809

Browse files
committed
fix: validate non-zero addresses for Code Scanning / Slither
- Require non-zero recipient/target on Call, SendingEther, TryCatch, Payable - Immutable constructor and Proxy / DelegatecallDemo delegate target - Add revert tests documenting the guardrails Made-with: Cursor
1 parent 1971e64 commit 0427809

13 files changed

Lines changed: 65 additions & 0 deletions

PROGRESS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ A hands-on Solidity training ground based on solidity-by-example.org.
77
## Completed
88

99
- [x] **Naming / NatSpec cleanup (2026-04)**: After Slither-oriented renames, fixed shadowing bugs (`value = value`, `num = num`), aligned bodies with new parameter names, removed drift-prone `/// @param` lines under `src/` (excluding `src/hacks/`), added `scripts/strip_natspec_params.py`, and updated tests (`ERC20Permit`, `GasGolf`, `Immutable`). Full suite green via `forge test` in Docker.
10+
- [x] **Slither / Code Scanning (zero address)**: Added `require(... != address(0), "Zero address")` on low-level call entry points flagged in PR review (`Call`, `SendingEther`, `TryCatch`, `Payable.withdrawTo`, `Immutable` constructor, `Delegatecall` proxy + demo), with matching revert tests.
1011

1112
### ✅ Phase 1: Project Setup
1213
- [x] Copy training documentation to repo

src/basic/Call.sol

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ contract Call {
1212

1313
/// @notice Call a function by selector
1414
function callBySelector(address target, bytes4 selector) external returns (bool, bytes memory) {
15+
require(target != address(0), "Zero address");
1516
(bool success, bytes memory data) = target.call(abi.encodePacked(selector));
1617
if (success) {
1718
emit CallSuccess(target, abi.encodePacked(selector), data);
@@ -23,13 +24,15 @@ contract Call {
2324

2425
/// @notice Call with specific value
2526
function callWithValue(address target, bytes calldata data, uint256 value) external payable returns (bool, bytes memory) {
27+
require(target != address(0), "Zero address");
2628
(bool success, bytes memory result) = target.call{value: value}(data);
2729
require(success, "Call failed");
2830
return (success, result);
2931
}
3032

3133
/// @notice Static call (no state changes)
3234
function staticCall(address target, bytes calldata data) external view returns (bool, bytes memory) {
35+
require(target != address(0), "Zero address");
3336
(bool success, bytes memory result) = target.staticcall(data);
3437
return (success, result);
3538
}
@@ -41,6 +44,7 @@ contract Call {
4144
bytes[] memory results = new bytes[](targets.length);
4245

4346
for (uint256 i = 0; i < targets.length; i++) {
47+
require(targets[i] != address(0), "Zero address");
4448
(bool success, bytes memory result) = targets[i].call(data[i]);
4549
successes[i] = success;
4650
results[i] = result;

src/basic/Delegatecall.sol

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ contract Proxy {
3131
address public implementation;
3232

3333
constructor(address _implementation) {
34+
require(_implementation != address(0), "Zero address");
3435
implementation = _implementation;
3536
owner = msg.sender;
3637
}
@@ -64,6 +65,7 @@ contract DelegatecallDemo {
6465

6566
/// @notice Execute delegatecall to target
6667
function executeDelegatecall(address target, uint256 newValue) external {
68+
require(target != address(0), "Zero address");
6769
(bool success,) = target.delegatecall(
6870
abi.encodeWithSignature("setValue(uint256)", newValue)
6971
);

src/basic/Immutable.sol

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ contract Immutable {
1919

2020
/// @notice Constructor sets immutable values
2121
constructor(uint256 initialUint, address initialAddress, bytes32 initialBytes32) {
22+
require(initialAddress != address(0), "Zero address");
2223
myUint = initialUint;
2324
myAddress = initialAddress;
2425
myBytes32 = initialBytes32;

src/basic/Payable.sol

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ contract Payable {
6161
/// @notice Withdraw to specific address
6262
function withdrawTo(address payable to, uint256 amount) external {
6363
require(msg.sender == owner, "Not owner");
64+
require(to != address(0), "Zero address");
6465
require(amount <= address(this).balance, "Insufficient balance");
6566
(bool success,) = to.call{value: amount}("");
6667
require(success, "Transfer failed");

src/basic/SendingEther.sol

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@ contract SendingEther {
1010

1111
/// @notice Send via transfer (reverts on failure, 2300 gas)
1212
function sendViaTransfer(address payable to, uint256 amount) external {
13+
require(to != address(0), "Zero address");
1314
to.transfer(amount);
1415
emit Sent(to, amount, "transfer");
1516
}
1617

1718
/// @notice Send via send (returns bool, 2300 gas)
1819
function sendViaSend(address payable to, uint256 amount) external returns (bool) {
20+
require(to != address(0), "Zero address");
1921
bool success = to.send(amount);
2022
require(success, "Send failed");
2123
emit Sent(to, amount, "send");
@@ -24,6 +26,7 @@ contract SendingEther {
2426

2527
/// @notice Send via call (forwards all gas, returns bool)
2628
function sendViaCall(address payable to, uint256 amount) external returns (bool) {
29+
require(to != address(0), "Zero address");
2730
(bool success,) = to.call{value: amount}("");
2831
require(success, "Call failed");
2932
emit Sent(to, amount, "call");
@@ -32,6 +35,7 @@ contract SendingEther {
3235

3336
/// @notice Recommended way: call with reentrancy protection
3437
function sendSafely(address payable to, uint256 amount) external {
38+
require(to != address(0), "Zero address");
3539
(bool success,) = to.call{value: amount}("");
3640
require(success, "Safe send failed");
3741
emit Sent(to, amount, "safe call");

src/basic/TryCatch.sol

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ contract TryCatch {
3434
ExternalContract public externalContract;
3535

3636
constructor(address _external) {
37+
require(_external != address(0), "Zero address");
3738
externalContract = ExternalContract(_external);
3839
}
3940

@@ -73,6 +74,7 @@ contract TryCatch {
7374

7475
/// @notice Try/catch low-level call
7576
function tryLowLevelCall(address target, bytes calldata data) external returns (bool, bytes memory) {
77+
require(target != address(0), "Zero address");
7678
(bool success, bytes memory result) = target.call(data);
7779
if (!success) {
7880
lastError = "Low-level call failed";

test/basic/Call.t.sol

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,5 +56,19 @@ contract CallTest is Test {
5656
assertEq(target.value(), 20); // Last call sets value to 20
5757
}
5858

59+
function test_Revert_callBySelector_zeroTarget() public {
60+
vm.expectRevert("Zero address");
61+
caller.callBySelector(address(0), bytes4(keccak256("getValue()")));
62+
}
63+
64+
function test_Revert_batchCall_zeroTarget() public {
65+
address[] memory targets = new address[](1);
66+
targets[0] = address(0);
67+
bytes[] memory data = new bytes[](1);
68+
data[0] = "";
69+
vm.expectRevert("Zero address");
70+
caller.batchCall(targets, data);
71+
}
72+
5973
receive() external payable {}
6074
}

test/basic/Delegatecall.t.sol

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,16 @@ contract DelegatecallTest is Test {
1717
demo = new DelegatecallDemo();
1818
}
1919

20+
function test_Revert_proxyConstructor_zeroImplementation() public {
21+
vm.expectRevert("Zero address");
22+
new Proxy(address(0));
23+
}
24+
25+
function test_Revert_executeDelegatecall_zeroTarget() public {
26+
vm.expectRevert("Zero address");
27+
demo.executeDelegatecall(address(0), 1);
28+
}
29+
2030
/// @notice Test delegatecall updates proxy storage
2131
function test_Delegatecall_UpdatesProxyStorage() public {
2232
// Call through proxy

test/basic/Immutable.t.sol

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ contract ImmutableTest is Test {
1616
immutableDefault = new ImmutableWithDefault();
1717
}
1818

19+
function test_Revert_constructor_zeroAddress() public {
20+
vm.expectRevert("Zero address");
21+
new Immutable(1, address(0), bytes32(0));
22+
}
23+
1924
/// @notice Unit test: myUint matches constructor arg
2025
function test_myUint_matchesConstructor() public view {
2126
assertEq(immutableContract.myUint(), 42);

0 commit comments

Comments
 (0)