OpenZeppelin / OpenZeppelin/stellar-contracts
🐞 [Bug]: weighted_threshold::install counts weights of addresses that are not rule signers
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 95
- Forks
- 68
- Avg merge
- 4d 42m
- Merged PRs (30d)
- 20
Description
What happened?
install in packages/accounts/src/policies/weighted_threshold.rs (v0.7.2, line 482) validates the threshold against the sum of all signer_weights:
let total_weight = calculate_total_weight(e, ¶ms.signer_weights);
if params.threshold == 0 || params.threshold > total_weight {
panic_with_error!(e, WeightedThresholdError::InvalidThreshold);
}
signer_weights is not checked against context_rule.signers. At enforcement, calculate_weight only counts authenticated signers of the rule, so weights assigned to non-members raise total_weight without ever being reachable.
A rule can therefore be installed, including from a constructor, with a threshold its own signers can never meet. Every authorization through that rule then fails with NotAllowed (3213). The same gap lets set_threshold and set_signer_weight accept thresholds that only non-member weights make reachable.
Reproduced on testnet with the unmodified examples/multisig-smart-account account and weighted threshold policy:
- Rule signers: 2 delegated signers with weight 1 each
signer_weightsalso contains a third address, not a rule signer, with weight 5threshold: 3, accepted byinstallbecause total weight is 7- Account
CASLFL4M7SKKPFO3FOJWRKJR62LPMYITCFNFQVLCXEEAU2DNTVECKMPY: a transfer signed by both rule signers fails withError(Contract, #3213)
Expected behavior
Either:
install(andset_threshold/set_signer_weight) compute the reachable weight fromcontext_rule.signersonly and reject thresholds above it; orinstallrejectssigner_weightsentries that are not signers of the rule, and the module docs state that weights must stay in sync with rule membership.
If the current behaviour is intended, a doc note next to the existing signer-management guidance would help integrators, similar to #761.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read packages/accounts/src/policies/weighted_threshold.rs, starting at install and the set_threshold and set_signer_weight paths, then compare their validation with calculate_weight. Use the multisig-smart-account example and the reported signer weights to reproduce the unreachable-threshold failure. Done means thresholds are validated against rule signers, or non-member weights are rejected, and authorization no longer fails because of unreachable weight.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- authorization, blockchain
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 72/100