critical: parameters multisig is decorative — admin bypass, stale approvals, self-updatable signers

[grantfox-oss]

Problem

Three flaws combine to make the parameters-contract multisig (contracts/parameters-contract/src/lib.rs) provide theater, not security:

1. Admin bypass: configure_multisig() (lines 41–57) requires only the single admin key's auth. The same admin retains do_set_admin() reachability through proposals AND direct set_admin-style power elsewhere in the protocol. A single compromised key can reconfigure or sidestep the entire "multisig".
2. Stale approvals survive signer-set changes: proposals store approvals: Vec<Address> (lines 79–91). If UpdateSigners executes and removes a signer, that removed signer's approval remains counted by execute() (line 132 checks only approvals.len() >= threshold). Revoked signers retain veto/exec power over in-flight proposals.
3. Self-serving threshold reduction: a signer set at threshold 2-of-3 can propose UpdateSigners to 2-of-2 (or admit a colluder); execute() validates against the CURRENT config, so the proposal needs only old-threshold approvals to install a weaker future threshold. There is no escalation guard requiring higher quorum for signer-set changes.

Combined: one colluding signer plus one stale approval can rewrite parameters (interest bps, min guarantee, grace periods) that directly control creditline economics.

Ground Rules

1. Read context/architecture-context.md, context/code-standards.md, context/progress-tracker.md in full
2. Read parameters-contract in full including storage.rs and types.rs (MultisigConfig, Proposal)
3. Preserve the existing event surface where possible; additive changes only

What To Build

1. Record the snapshot of eligible signer addresses (not just a count) at proposal time; approve() and execute() must validate each approver against the snapshot AND current membership, rejecting anyone removed since.
2. Require a strictly higher quorum for UpdateSigners actions (e.g. threshold + 1 or full unanimity — pick and document one) so signers cannot cheapen their own gate.
3. On configure_multisig, emit a prominent event and consider a two-step confirmation (propose→confirm) so a single admin key cannot silently swap the signer set.
4. Clear/re-validate in-flight proposals whose action targets the signer set when the signer set changes.
5. Tests for every branch above, including the stale-approval exploit reproducing end-to-end before the fix and failing after.

Files To Touch

• contracts/parameters-contract/src/lib.rs
• contracts/parameters-contract/src/storage.rs
• contracts/parameters-contract/src/types.rs
• contracts/parameters-contract/src/tests.rs
• context/progress-tracker.md

Acceptance Criteria

• Removed signers' approvals are never counted
• Signer-set changes require elevated quorum
• Exploit reproduction test exists and fails pre-fix / passes post-fix semantics documented in the PR
• Build zero errors, all tests green

Mandatory Checks Before Opening PR

• All context/ files read and understood
• Code follows context/code-standards.md exactly
• progress-tracker updated
• New tests added; suite green
• PR references this issue number exactly

PRs failing any check will be closed without review.

[angelraph]

Hi, I’d like to work on this issue.

I’ve read through the scope, and I understand the three security problems being addressed here: approvals surviving signer-set changes, signer-set updates being able to weaken the future quorum, and the current admin authorization allowing the multisig configuration to be changed without the intended security guarantees.

My approach would be to first trace the existing "MultisigConfig", "Proposal", storage, approval, execution, and admin flows so the fix preserves the contract’s current behavior and event surface where possible.

For the implementation, I’ll:

• Snapshot the eligible signer addresses when a proposal is created, rather than relying only on the approval count.
• Validate approvals against both the proposal snapshot and the current signer set, so removed signers can’t retain authority through an in-flight proposal.
• Add an elevated quorum requirement specifically for "UpdateSigners" actions and document the chosen rule clearly.
• Revisit "configure_multisig()" and the signer-set update lifecycle so a single admin authorization cannot silently undermine the multisig guarantees.
• Revalidate or invalidate affected in-flight proposals whenever the signer configuration changes.
• Add regression tests that reproduce the stale-approval and quorum-reduction cases, along with coverage for the relevant authorization and proposal branches.
• Run the required contract checks and update "context/progress-tracker.md" as part of the PR.

I’ll keep the changes scoped to the listed files and follow the repository’s existing contract patterns rather than introducing a separate multisig design.

The main goal is to make the security properties enforceable by the contract itself, with tests demonstrating that the previously exploitable states can no longer be executed.

[dimin4241-svg]

Hi, I’d like to take this on. I work with Rust/Soroban state and authorization logic, and I understand the fix needs membership-valid approvals, elevated quorum for signer changes, and safe handling of in-flight proposals without breaking existing events. I can deliver a focused patch with exploit-regression tests within 24 hours. Please assign me.

[KingFRANKHOOD]

Hi Team, I would love to take on this issue. Hardening the parameters-contract multisig against admin bypasses, stale approvals, and self-serving threshold reductions is critical for maintaining true protocol governance security.Before writing code, I will review context/architecture-context.md, context/code-standards.md, and context/progress-tracker.md to ensure complete alignment with established contract standards and storage layouts.To address these vulnerabilities:Stale Approval Invalidation: In storage.rs and lib.rs, proposals will record the active signer set snapshot at creation time. During approve() and execute(), each approval will be strictly re-validated against both the creation snapshot AND current signer membership, immediately disqualifying any removed signers.Elevated Quorum for Signer Changes: I will enforce a strictly higher quorum requirement (such as full unanimity of the active signer set) for any UpdateSigners proposal, preventing a sub-group from lowering their own execution barrier.Proposal Invalidation & Re-configuration Protection: Executing an UpdateSigners action will automatically clear or invalidate in-flight proposals targeting signer-set changes. Additionally, configure_multisig will emit a prominent event and utilize a two-step confirmation flow to ensure single-key admin bypasses cannot silently replace signers.Comprehensive Testing: In src/tests.rs, I will write an exploit reproduction test demonstrating the stale approval vulnerability pre-fix and proving its resolution post-fix, alongside tests for elevated quorums and invalidations. Finally, I will update context/progress-tracker.md.With strong experience in Rust, Soroban contract security, and multisig governance primitives, I am ready to deliver a clean implementation. Please assign this issue to me!

[Yasir-TechGuy]

Please assign this issue to me

[grantfox-oss]

🦊 GrantFox — @KingFRANKHOOD has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #107)
2. Your PR will be reviewed by the StepFi-app maintainers

Good luck! Track your progress on GrantFox.