Skip to content

reward-engine set_token and set_registry have no validation; swapping to an uninitialised contract address silently breaks the engine #32

Description

@cybermax4200

Labels: type: bug, type: security, difficulty: intermediate, area: reward-engine

Why this matters now

set_token and set_registry are admin-callable functions that redirect all future cross-contract calls to a new address. If the admin provides a wrong address (e.g., an uninitialised contract, a user wallet, or a typo), every subsequent approve_proof call will panic at the cross-contract call site with a cryptic SDK error — not a clean engine error. There is no on-chain event emitted, no validation, and no rollback if the new address turns out to be broken.

Problem / What

set_token and set_registry in verification.rs:

pub fn set_token(e: Env, caller: Address, new_token: Address) {
    caller.require_auth();
    require_admin(&e, &caller);
    storage::write_token(&e, &new_token);
}

No event. No validation that new_token != current_token. No validation that new_token is a contract address (though Soroban addresses are typed, a user account address is accepted). No two-step pattern.

Additionally, set_registry does not check that the engine is registered as a sponsor on the new registry — if it isn't, every subsequent complete_task call will fail with "registry: unauthorized".

Key Challenges

  • Soroban does not provide an on-chain way to call a "probe" function before committing the change; the best approach is a try-call or an explicit version check call in the setter.
  • A TokenUpdatedEvent and RegistryUpdatedEvent must be added.
  • The same-address guard (new_token == current_token) is a simple no-op protection.

Acceptance Criteria

  • set_token emits TokenUpdatedEvent { admin, previous_token, new_token } and panics if new_token == current_token.
  • set_registry emits RegistryUpdatedEvent { admin, previous_registry, new_registry } and panics if new_registry == current_registry.
  • Tests cover: successful update with event assertion, same-address no-op panic, unauthorized caller panic.
  • README updated to document that after set_registry the caller must ensure the engine is re-added as a sponsor on the new registry.
  • All existing test_set_token and test_set_registry tests pass or are updated.

Relevant files / functions

File Symbol
contracts/reward-engine/src/verification.rs set_token, set_registry

Out of scope

  • Two-step pattern for set_token/set_registry (covered by Issue 16 pattern; optional extension)
  • Changes to set_oracle

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardThird CampaignCampaign: Third CampaignbugSomething isn't workingsecurity

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions