Skip to content

security: Require genuine admin auth on DAO config setters #871

Description

@Samuel1505

Description

// dao.rs:12-41
pub fn set_dao_mode(env: &Env, admin: &Address, enabled: bool) -> Result<(), ContractError> {
    require_global_admin(env, admin)?;
    Storage::set_dao_mode_enabled(env, enabled);
    Ok(())
}

pub fn set_voting_period(env: &Env, admin: &Address, ledgers: u32) -> Result<(), ContractError> {
    require_global_admin(env, admin)?;
    ...
}

pub fn set_quorum_votes(env: &Env, admin: &Address, quorum: u64) -> Result<(), ContractError> {
    require_global_admin(env, admin)?;
    ...
}
...
fn require_global_admin(env: &Env, caller: &Address) -> Result<(), ContractError> {
    let admin = Storage::get_global_admin(env).ok_or(ContractError::Unauthorized)?;
    if admin != *caller {
        return Err(ContractError::Unauthorized);
    }
    Ok(())
}

require_global_admin only compares admin != *caller — it never calls .require_auth() on admin/caller. Neither these three module functions nor their lib.rs entrypoints (dao_set_mode, dao_set_voting_period, dao_set_quorum_votes) invoke require_auth() anywhere in the chain. Since the global admin's address is public (visible in any of its prior signed transactions/events), anyone can pass it as the admin parameter and pass this check without ever signing anything.

Every other admin-setter in this codebase (treasury.rs, compliance.rs, metrics.rs, migration.rs, whitelist.rs, hooks.rs, insurance.rs, grant_bridge.rs, escrow.rs, multisig.rs, matching.rs, crowdfund.rs, revenue_share.rs, arbitration_pool.rs) precedes its admin == stored_admin check with a genuine .require_auth() either in lib.rs or inside the module. dao.rs's three setters are the sole exception, strongly suggesting this is an oversight.

Impact: combined with dao::create_proposal having no eligibility gate (see issue #906), an attacker can call set_quorum_votes(admin_addr, 1) and set_voting_period(admin_addr, 1) — reducing quorum to 1 and the voting window to a single ledger — then single-handedly pass and execute a ChangeAdmin or TreasuryWithdrawal proposal, all without the real admin ever signing anything.

Technical Requirements

Files to update

  • contracts/contracts/stellar-grants/src/dao.rs (set_dao_mode, lines 12-16; set_voting_period, lines 23-30; set_quorum_votes, lines 34-41; require_global_admin, lines 231-237)

Fix direction

Add admin.require_auth() inside require_global_admin (or immediately before each call site) so the equality check is backed by a genuine signature:

fn require_global_admin(env: &Env, caller: &Address) -> Result<(), ContractError> {
    caller.require_auth();
    let admin = Storage::get_global_admin(env).ok_or(ContractError::Unauthorized)?;
    if admin != *caller {
        return Err(ContractError::Unauthorized);
    }
    Ok(())
}

Since require_global_admin is shared by all three setters, this one change fixes all of them.

Acceptance Criteria

  • set_dao_mode/set_voting_period/set_quorum_votes cannot be called successfully by passing the admin's address as a parameter without the admin actually authorizing the call.
  • A test confirms a stranger who supplies the real admin's address (without mocking/providing that admin's auth) is rejected.
  • The legitimate admin flow continues to work and is covered by a test.
  • cargo test passes.

Estimated Effort

Beginner: 3 hours
Intermediate: 1.5 hours
Expert: 1 hour

How to work this issue

  1. Read contracts/ContributionGuide.md for the contribution workflow.
  2. Comment on the issue to claim it before starting.
  3. Branch: fix/issue-926-dao-admin-setter-auth.
  4. Run cargo fmt, cargo clippy -- -D warnings, cargo test before opening your PR.
  5. Use a Conventional Commit message, e.g. security: require genuine admin auth on DAO config setters.

Before you start

If you find this project interesting, please consider starring the repository on GitHub. It helps the project gain visibility and supports the Drips Wave program that rewards contributors for merged fixes like this one.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions