Skip to content

fix(proxy): add zero address validation for proxy parameter #6257

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.

Sign up for GitHub

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

Closed
DeVikingMark wants to merge 1 commit into from
+9 −0
Closed

fix(proxy): add zero address validation for proxy parameter #6257

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions contracts/proxy/transparent/ProxyAdmin.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ import {Ownable} from "../../access/Ownable.sol";
* explanation of why you would want to use this see the documentation for {TransparentUpgradeableProxy}.
*/
contract ProxyAdmin is Ownable {
/**
* @dev The `proxy` address is invalid (e.g. `address(0)`).
*/
error ProxyAdminInvalidProxy(address proxy);

/**
* @dev The version of the upgrade interface of the contract. If this getter is missing, both `upgrade(address,address)`
* and `upgradeAndCall(address,address,bytes)` are present, and `upgrade` must be used if no function should be called,
Expand All @@ -32,6 +37,7 @@ contract ProxyAdmin is Ownable {
*
* Requirements:
*
* - `proxy` must not be the zero address.
* - This contract must be the admin of `proxy`.
* - If `data` is empty, `msg.value` must be zero.
*/
Expand All @@ -40,6 +46,9 @@ contract ProxyAdmin is Ownable {
address implementation,
bytes memory data
) public payable virtual onlyOwner {
if (address(proxy) == address(0)) {
revert ProxyAdminInvalidProxy(address(0));
}
Comment on lines +49 to +51
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

fd -e test.js -e test.ts -e spec.js -e spec.ts | grep -i proxy | head -20

Repository: OpenZeppelin/openzeppelin-contracts

Length of output: 447

🏁 Script executed:

rg "ProxyAdminInvalidProxy" --type=js --type=ts -l

Repository: OpenZeppelin/openzeppelin-contracts

Length of output: 61

🏁 Script executed:

rg "upgradeAndCall" --type=js --type=ts -l | grep test

Repository: OpenZeppelin/openzeppelin-contracts

Length of output: 120

🏁 Script executed:

cat -n test/proxy/transparent/ProxyAdmin.test.js | head -150

Repository: OpenZeppelin/openzeppelin-contracts

Length of output: 3882

🏁 Script executed:

rg "upgradeAndCall|ProxyAdminInvalidProxy" test/proxy/transparent/ProxyAdmin.test.js -B 2 -A 5

Repository: OpenZeppelin/openzeppelin-contracts

Length of output: 1802

🏁 Script executed:

cat -n contracts/proxy/transparent/ProxyAdmin.sol | sed -n '40,60p'

Repository: OpenZeppelin/openzeppelin-contracts

Length of output: 707

🏁 Script executed:

rg "ProxyAdminInvalidProxy" contracts/proxy/transparent/ProxyAdmin.sol -B 2 -A 2

Repository: OpenZeppelin/openzeppelin-contracts

Length of output: 433

Add test coverage for zero address validation.

The defensive check is well-implemented, but the test file lacks coverage for the ProxyAdminInvalidProxy error when a zero address is passed. Add a test case to verify that upgradeAndCall reverts with ProxyAdminInvalidProxy when the proxy address is address(0).

🤖 Prompt for AI Agents 
In contracts/proxy/transparent/ProxyAdmin.sol around lines 49-51, add a unit
test that calls upgradeAndCall with proxy address set to address(0) and expects
it to revert with the custom error ProxyAdminInvalidProxy; create or update the
ProxyAdmin test (e.g., test/ProxyAdmin.test.ts) to invoke
proxyAdmin.upgradeAndCall(ethers.constants.AddressZero, implAddress, callData)
and assert using Chai/Hardhat helpers: await
expect(<call>).to.be.revertedWithCustomError(proxyAdmin,
"ProxyAdminInvalidProxy").withArgs(ethers.constants.AddressZero), ensuring the
test supplies any needed mocks/fixtures for proxyAdmin and implementation
addresses.

proxy.upgradeToAndCall{value: msg.value}(implementation, data);
}
}
Loading