BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: medium
Likelihood: low
Invalid

An empty-data UUPS upgrade installs an implementation nothing has ever `DELEGATECALL`ed, permanently disabling the factory

Author Revealed upon completion

Description

  • ConfidencePoolFactory is a UUPS proxy: every call reaches its implementation via DELEGATECALL. Nothing in the upgrade path verifies that the proxy can actually DELEGATECALL the candidate.

  • _authorizeUpgrade checks the caller, not the candidate. The inherited UUPS check calls proxiableUUID() on the candidate - a plain CALL - and verifies the returned slot. The only step that would exercise DELEGATECALL is skipped entirely when data is empty:

// ConfidencePoolFactory - authorizes the owner, not the candidate
@> function _authorizeUpgrade(address) internal override onlyOwner {}
​
// OZ ERC1967Utils.upgradeToAndCall:67-76 - empty data skips the only delegate-call smoke test
_setImplementation(newImplementation); // slot written first
emit IERC1967.Upgraded(newImplementation);
if (data.length > 0) {
Address.functionDelegateCall(newImplementation, data); // never reached on empty data
} else {
@> _checkNonPayable();
}

  • So an implementation that answers proxiableUUID() but cannot be delegatecalled passes every check, is installed, and then every path through the proxy reverts - createPool, the setters, pause, and upgradeToAndCall itself. Because recovery runs through the same dead DELEGATECALL, the proxy cannot be rolled back: a replacement factory must be deployed.

  • One concrete way this arises here. BattleChain is "a ZK rollup built on ZKsync OS" (BattleChain docs), and ZKsync OS is specified as multi-environment: "ZKsync OS will include the following EEs: EVM... WASM... Native RISC V" (ZKsync docs). ZKsync documents delegation being forbidden across environment boundaries: "EVM contracts cannot use delegatecall to EraVM contracts. EraVM contracts cannot use delegatecall to EVM contracts." (ZKsync docs).

Stated precisely: that quote documents the EVM/EraVM boundary on ZKsync Era. Whether ZKsync OS applies the same restriction to its WASM / RISC-V environments, and whether any non-EVM environment is live on BattleChain today, is not established by these docs - both EEs are described as planned. That bounds the likelihood, not the defect: the missing verification is unconditional, and any un-delegatecall-able implementation triggers it.

Risk

Severity: Medium - Medium Impact x Low Likelihood.

Likelihood: Low. The owner must upgrade to an implementation the EVM proxy cannot DELEGATECALL, using empty calldata. Standard solc EVM upgrades are unaffected; the risk is operational upgrades that mix execution environments, and the non-EVM environments above are stated as planned rather than confirmed live on BattleChain today.

Impact: Medium. The only factory proxy is permanently disabled - createPool, config, pause and the UUPS recovery entrypoint all revert - and cannot be restored through UUPS, because recovery uses the same broken DELEGATECALL. Existing ConfidencePool clones are non-upgradeable and execute independently, so staker funds are not directly at risk.

Proof of Concept

Self-contained: drop into test/ and run. Both tests PASS, no RPC. The second confirms the fix, non-empty calldata forces the delegate call, the candidate reverts, and the upgrade rolls back with the slot unchanged.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Test} from "forge-std/Test.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
​
import {ConfidencePool} from "src/ConfidencePool.sol";
import {ConfidencePoolFactory} from "src/ConfidencePoolFactory.sol";
import {MockERC20} from "test/mocks/MockERC20.sol";
import {MockAgreement} from "test/mocks/MockAgreement.sol";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
​
/// @dev Stands in for an implementation the EVM proxy cannot DELEGATECALL. Pure EVM cannot deploy
/// non-EVM bytecode, so the `address(this) == _self` guard models the environment boundary: it
/// answers OZ's `proxiableUUID()` probe on a direct CALL, but reverts on every proxy DELEGATECALL.
contract NativeIncompatibleImpl {
address private immutable _self = address(this);
bytes32 private constant _ERC1967_IMPL_SLOT =
0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
​
function proxiableUUID() external view returns (bytes32) {
require(address(this) == _self, "native EE: only direct call");
return _ERC1967_IMPL_SLOT;
}
​
fallback() external {
revert("native EE: cross-environment DELEGATECALL reverts");
}
}
​
contract M2UpgradeBrickStandaloneTest is Test {
uint256 internal constant ONE = 1e18;
address internal constant SCOPE_ACCOUNT = address(0xC0FFEE);
bytes32 internal constant IMPL_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
​
MockERC20 internal token;
MockSafeHarborRegistry internal registry;
MockAgreement internal agreement;
ConfidencePoolFactory internal factory;
​
address internal agreementOwner = makeAddr("agreementOwner");
address internal recovery = makeAddr("recovery");
​
function setUp() public {
token = new MockERC20();
registry = new MockSafeHarborRegistry();
agreement = new MockAgreement(agreementOwner);
agreement.setContractInScope(SCOPE_ACCOUNT, true);
registry.setAgreementValid(address(agreement), true);
​
ConfidencePool poolImpl = new ConfidencePool();
ERC1967Proxy proxy = new ERC1967Proxy(
address(new ConfidencePoolFactory()),
abi.encodeCall(
ConfidencePoolFactory.initialize,
(address(registry), address(poolImpl), makeAddr("moderator"))
)
);
factory = ConfidencePoolFactory(address(proxy));
factory.setStakeTokenAllowed(address(token), true);
}
​
function test_emptyDataUpgrade_permanentlyBricksFactory() external {
vm.prank(agreementOwner);
factory.createPool(address(agreement), address(token), block.timestamp + 31 days, ONE, recovery, _scope());
address goodImpl = _implementationOf(address(factory));
​
// Empty data: proxiableUUID() passes the probe, ERC1967Utils skips the delegatecall smoke
// test, and the un-delegatecall-able impl is installed.
NativeIncompatibleImpl nativeV2 = new NativeIncompatibleImpl();
factory.upgradeToAndCall(address(nativeV2), "");
assertEq(_implementationOf(address(factory)), address(nativeV2));
​
vm.prank(agreementOwner);
vm.expectRevert();
factory.createPool(address(agreement), address(token), block.timestamp + 31 days, ONE, recovery, _scope());
​
vm.expectRevert();
factory.owner();
​
// Recovery is impossible: upgradeToAndCall is itself behind the dead proxy DELEGATECALL.
vm.expectRevert();
factory.upgradeToAndCall(goodImpl, "");
assertEq(_implementationOf(address(factory)), address(nativeV2));
}
​
function test_nonEmptyValidationData_rejectsIncompatibleImplAtomically() external {
address goodImpl = _implementationOf(address(factory));
NativeIncompatibleImpl nativeV2 = new NativeIncompatibleImpl();
​
// Non-empty data forces the delegatecall, which reverts and rolls the upgrade back.
vm.expectRevert();
factory.upgradeToAndCall(address(nativeV2), abi.encodeWithSignature("postUpgradeCheck(uint256)", uint256(2)));
assertEq(_implementationOf(address(factory)), goodImpl);
​
vm.prank(agreementOwner);
factory.createPool(address(agreement), address(token), block.timestamp + 31 days, ONE, recovery, _scope());
}
​
function _scope() internal pure returns (address[] memory accounts) {
accounts = new address[](1);
accounts[0] = SCOPE_ACCOUNT;
}
​
function _implementationOf(address proxy) internal view returns (address) {
return address(uint160(uint256(vm.load(proxy, IMPL_SLOT))));
}
}

Recommended Mitigation

Reject empty-data upgrades, forcing the proxy to exercise DELEGATECALL - and letting the new implementation validate its own version/storage - before the upgrade can commit:

+error EmptyUpgradeValidationData();
+
+function upgradeToAndCall(address newImplementation, bytes memory data) public payable override onlyProxy {
+ if (data.length == 0) revert EmptyUpgradeValidationData();
+ super.upgradeToAndCall(newImplementation, data);
+}

Deployment tooling then passes a versioned validation call (e.g. abi.encodeCall(ConfidencePoolFactoryV2.postUpgradeCheck, (EXPECTED_VERSION))), and the runbook should require implementations be simulated against BattleChain testnet before mainnet

Updates

Lead Judging Commences

inallhonesty Lead Judge
16 days ago
inallhonesty Lead Judge 2 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

QA / Informational / Gas

inallhonesty Lead Judge 2 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

QA / Informational / Gas

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!