Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions DUMMY.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
dummy
10 changes: 10 additions & 0 deletions src/foundation/Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
// ========================================================================
Expand Down Expand Up @@ -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
// ========================================================================
Expand Down
7 changes: 7 additions & 0 deletions src/governance/Governance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
23 changes: 22 additions & 1 deletion src/oracle/jwk/JWKManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions src/runtime/GovernanceConfig.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions test/unit/governance/GovernanceConfig.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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()
);
Expand Down
Loading