diff --git a/DUMMY.txt b/DUMMY.txt new file mode 100644 index 0000000..421376d --- /dev/null +++ b/DUMMY.txt @@ -0,0 +1 @@ +dummy diff --git a/src/foundation/Errors.sol b/src/foundation/Errors.sol index c59f5c8..487d132 100644 --- a/src/foundation/Errors.sol +++ b/src/foundation/Errors.sol @@ -315,6 +315,12 @@ library Errors { /// @param proposalId ID of the unresolved proposal error ProposalNotResolved(uint64 proposalId); + /// @notice A proposal cannot target the Governance contract itself. + /// @dev Prevents a passing proposal from calling transferOwnership / addExecutor / removeExecutor + /// on Governance via execute(), which would escalate or transfer control. + /// @param index Index of the offending target in the proposal batch + error ProposalTargetsGovernance(uint256 index); + // ======================================================================== // TIMESTAMP ERRORS // ======================================================================== @@ -534,6 +540,10 @@ library Errors { /// @param count The total number of providers error JWKProviderIndexOutOfBounds(uint256 index, uint256 count); + /// @notice JWK failed structural validation (empty issuer, kid, modulus, or exponent) + /// @param issuer The issuer URL of the offending entry + error InvalidJWK(bytes issuer); + // ======================================================================== // ROLE CHANGE ERRORS // ======================================================================== diff --git a/src/governance/Governance.sol b/src/governance/Governance.sol index 03d5330..ba44076 100644 --- a/src/governance/Governance.sol +++ b/src/governance/Governance.sol @@ -526,6 +526,13 @@ contract Governance is IGovernance, Ownable2Step { // Execute all calls atomically uint256 len = targets.length; for (uint256 i = 0; i < len; ++i) { + // Defense-in-depth: forbid a proposal from calling Governance on itself. + // Without this, a passing proposal could invoke transferOwnership / addExecutor / + // removeExecutor (which are onlyOwner) whenever Governance is its own owner, + // escalating to full control of proposal execution. + if (targets[i] == address(this)) { + revert Errors.ProposalTargetsGovernance(i); + } (bool success, bytes memory returnData) = targets[i].call(datas[i]); if (!success) { revert Errors.ExecutionFailed(proposalId, returnData); diff --git a/src/oracle/jwk/JWKManager.sol b/src/oracle/jwk/JWKManager.sol index 98eae5e..214c4c8 100644 --- a/src/oracle/jwk/JWKManager.sol +++ b/src/oracle/jwk/JWKManager.sol @@ -78,6 +78,27 @@ contract JWKManager is IJWKManager, IOracleCallback { for (uint256 i; i < length;) { bytes memory issuer = issuers[i]; + if (issuer.length == 0) { + revert Errors.InvalidJWK(issuer); + } + + // Structural validation: downstream signature verification assumes non-empty kid/e/n. + // A malformed genesis entry would silently persist and poison auth lookups forever. + RSA_JWK[] calldata issuerJwks = jwks[i]; + uint256 jwkLen = issuerJwks.length; + for (uint256 j; j < jwkLen;) { + RSA_JWK calldata jwk = issuerJwks[j]; + if ( + bytes(jwk.kid).length == 0 || bytes(jwk.kty).length == 0 || bytes(jwk.alg).length == 0 + || bytes(jwk.e).length == 0 || bytes(jwk.n).length == 0 + ) { + revert Errors.InvalidJWK(issuer); + } + unchecked { + ++j; + } + } + bytes32 issuerHash = keccak256(issuer); // Initial version is 1 for genesis validators @@ -89,7 +110,7 @@ contract JWKManager is IJWKManager, IOracleCallback { // Upsert into observed JWKs _upsertObservedProvider(issuerHash, issuer, version, jwks[i]); - emit ObservedJWKsUpdated(issuer, version, jwks[i].length); + emit ObservedJWKsUpdated(issuer, version, jwkLen); unchecked { ++i; diff --git a/src/runtime/GovernanceConfig.sol b/src/runtime/GovernanceConfig.sol index 6256b65..de7950c 100644 --- a/src/runtime/GovernanceConfig.sol +++ b/src/runtime/GovernanceConfig.sol @@ -200,8 +200,10 @@ contract GovernanceConfig { if (_requiredProposerStake == 0) { revert Errors.InvalidProposerStake(); } - // Prevent setting proposer stake to type(uint256).max which would block all proposals - if (_requiredProposerStake == type(uint256).max) { + // Pool voting power is clamped to uint128 in Governance.getRemainingVotingPower, so any + // requiredProposerStake above uint128.max is unreachable — effectively blocking all new + // proposals and freezing governance. Reject such values. + if (_requiredProposerStake > type(uint128).max) { revert Errors.ExcessiveRequiredProposerStake(); } if (_votingDurationMicros < MIN_VOTING_DURATION) { diff --git a/test/unit/governance/GovernanceConfig.t.sol b/test/unit/governance/GovernanceConfig.t.sol index c1b7cc2..8f6d634 100644 --- a/test/unit/governance/GovernanceConfig.t.sol +++ b/test/unit/governance/GovernanceConfig.t.sol @@ -266,7 +266,7 @@ contract GovernanceConfigTest is Test { uint64 votingDurationMicros ) public { vm.assume(minVotingThreshold > 0 && minVotingThreshold < type(uint128).max); - vm.assume(requiredProposerStake > 0 && requiredProposerStake < type(uint256).max); + vm.assume(requiredProposerStake > 0 && requiredProposerStake <= type(uint128).max); vm.assume(votingDurationMicros >= GOV_MIN_VOTING_DURATION && votingDurationMicros <= GOV_MAX_VOTING_DURATION); vm.prank(SystemAddresses.GENESIS); @@ -285,7 +285,7 @@ contract GovernanceConfigTest is Test { _initializeConfig(); vm.assume(minVotingThreshold > 0 && minVotingThreshold < type(uint128).max); - vm.assume(requiredProposerStake > 0 && requiredProposerStake < type(uint256).max); + vm.assume(requiredProposerStake > 0 && requiredProposerStake <= type(uint128).max); vm.assume( votingDurationMicros >= config.MIN_VOTING_DURATION() && votingDurationMicros <= config.MAX_VOTING_DURATION() );