security: Enforce whitelist, dedup, and max-reviewers cap in accept_request

[Samuel1505]

Description

// reviewer_pool.rs:112-157
pub fn accept_request(env: &Env, reviewer: &Address, grant_id: u64) -> Result<(), ContractError> {
    reviewer.require_auth();
    let request = Storage::get_reviewer_request(env, grant_id, reviewer).ok_or(ContractError::InvalidState)?;
    if request.status != ReviewerRequestStatus::Pending { return Err(ContractError::InvalidState); }
    if env.ledger().timestamp() > request.expires_at { return Err(ContractError::InvalidState); }
    let profile = Storage::get_reviewer_profile(env, reviewer).ok_or(ContractError::InvalidState)?;
    if profile.availability != ReviewerAvailability::Available { return Err(ContractError::InvalidState); }

    let mut grant = Storage::get_grant_v(env, grant_id);
    grant.reviewers.push_back(reviewer.clone());
    Storage::set_grant(env, grant_id, &grant);
    ...
}

This pushes reviewer onto grant.reviewers with no whitelist check and no cap check. Compare with the two other places grant.reviewers gains an entry:

• internal_grant_create (in lib.rs) enforces whitelist::is_allowed(env, &r, &WhitelistScope::GlobalReviewer) and rejects once reviewers.len() > protocol_cfg.max_reviewers.
• multi_grant::add_reviewer_to_grant explicitly re-checks the whitelist and dedups via .contains().

accept_request has neither check, and there's also no dedup: request_reviewer can be called again for the same (grant_id, reviewer) pair (it unconditionally overwrites the stored request back to Pending), and accept_request never checks grant.reviewers.contains(reviewer) before pushing — so the same reviewer can be accepted multiple times, growing grant.reviewers past protocol_cfg.max_reviewers, an invariant enforced everywhere else at grant creation but silently bypassable here.

Exploit: An admin sets WhitelistScope::GlobalReviewer to Restricted specifically to control who can review grants (as enforced by internal_grant_create/multi_grant::add_reviewer_to_grant). A non-whitelisted reviewer registers via reviewer_register, the grant owner calls reviewer_request for them (or the reviewer requests to review, depending on the direction of request_reviewer), and the reviewer calls reviewer_accept_request — bypassing the whitelist entirely via a different code path than the ones that check it.

Technical Requirements

Files to update

• contracts/contracts/stellar-grants/src/reviewer_pool.rs (accept_request, lines 112-157)

Fix direction

pub fn accept_request(env: &Env, reviewer: &Address, grant_id: u64) -> Result<(), ContractError> {
    reviewer.require_auth();
    let request = Storage::get_reviewer_request(env, grant_id, reviewer).ok_or(ContractError::InvalidState)?;
    if request.status != ReviewerRequestStatus::Pending { return Err(ContractError::InvalidState); }
    if env.ledger().timestamp() > request.expires_at { return Err(ContractError::InvalidState); }
    let profile = Storage::get_reviewer_profile(env, reviewer).ok_or(ContractError::InvalidState)?;
    if profile.availability != ReviewerAvailability::Available { return Err(ContractError::InvalidState); }

    if !crate::whitelist::is_allowed(env, reviewer, &WhitelistScope::GlobalReviewer) {
        return Err(ContractError::Unauthorized);
    }

    let mut grant = Storage::get_grant_v(env, grant_id);
    if grant.reviewers.contains(reviewer.clone()) {
        return Err(ContractError::AlreadyRegistered);
    }
    let protocol_cfg = Storage::get_protocol_config(env);
    if grant.reviewers.len() >= protocol_cfg.max_reviewers {
        return Err(ContractError::InvalidInput);
    }
    grant.reviewers.push_back(reviewer.clone());
    Storage::set_grant(env, grant_id, &grant);
    // ... rest unchanged
}

Adjust the exact whitelist/config accessor calls to match whatever helper functions internal_grant_create/multi_grant::add_reviewer_to_grant already use, for consistency.

Acceptance Criteria

• A non-whitelisted reviewer (when GlobalReviewer scope is Restricted) cannot become a grant reviewer via accept_request.
• accept_request cannot push the same reviewer onto grant.reviewers twice, and cannot exceed protocol_cfg.max_reviewers.
• A test confirms all three invariants (whitelist, dedup, cap) are enforced through this path specifically.
• cargo test passes.

Estimated Effort

Beginner: 4 hours
Intermediate: 2 hours
Expert: 1.5 hours

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-928-reviewer-pool-accept-request-whitelist.
4. Run cargo fmt, cargo clippy -- -D warnings, cargo test before opening your PR.
5. Use a Conventional Commit message, e.g. security: enforce whitelist, dedup, and max-reviewers cap in accept_request.

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.