OpenZeppelin / OpenZeppelin/stellar-contracts

🐞 [Bug]: weighted_threshold::install counts weights of addresses that are not rule signers

Open
#891 0 comments 0 reactions 0 assignees View on GitHub

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, &params.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_weights also contains a third address, not a rule signer, with weight 5
  • threshold: 3, accepted by install because total weight is 7
  • Account CASLFL4M7SKKPFO3FOJWRKJR62LPMYITCFNFQVLCXEEAU2DNTVECKMPY: a transfer signed by both rule signers fails with Error(Contract, #3213)
Expected behavior

Either:

  1. install (and set_threshold / set_signer_weight) compute the reachable weight from context_rule.signers only and reject thresholds above it; or
  2. install rejects signer_weights entries 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.