Skip to content

fix: close unauthenticated admin claim in set_admin - #103

Merged
EmeditWeb merged 1 commit into
StepFi-app:mainfrom
onajidavid87-web:close-unauthenticated-admin-claim-branch-
Aug 29, 2026
Merged

fix: close unauthenticated admin claim in set_admin#103
EmeditWeb merged 1 commit into
StepFi-app:mainfrom
onajidavid87-web:close-unauthenticated-admin-claim-branch-

Conversation

@onajidavid87-web

Copy link
Copy Markdown
Contributor

Summary

Closes #80

This PR resolves a critical security vulnerability in reputation-contract's set_admin() function where an unauthenticated fallback branch allowed anyone to claim admin rights on an uninitialized contract. It introduces an explicit, single-use initialize(env, admin) entrypoint requiring admin.require_auth() and checks has_admin to prevent re-initialization with AlreadyInitialized. set_admin() is updated to require existing admin authorization (old_admin.require_auth()) and fails with NotInitialized if called before initialization. Additionally, comprehensive security unit tests were added, and deployment scripts, testnet metadata, and integration test setups were updated accordingly.

This repo is for Soroban smart contracts only

Before submitting, confirm your changes belong here:

  • My changes are inside the contracts/ directory
  • I have NOT added any TypeScript, React, or frontend files
  • I have NOT added package.json, vite.config.ts, index.html, or any Node.js files
  • My changes are written in Rust

Type of change

  • Bug fix
  • New contract function
  • Test coverage
  • Storage or type changes
  • Upgrade/migration utility

Testing

  • cargo build passes with zero errors
  • cargo test passes — all existing tests still pass (367 passed across workspace)
  • New tests written for every new function (5 new security tests for initialize & set_admin)
  • require_auth() is first line of every mutating function I added or changed
  • extend_ttl() called after every persistent storage write I added or changed
  • No .unwrap() or .expect() in user-facing paths

Context files reviewed

  • context/architecture-context.md
  • context/code-standards.md
  • context/progress-tracker.md updated

Mandatory before requesting review

Running these must all exit 0:
cargo build
cargo test -p reputation-contract

@EmeditWeb EmeditWeb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Automated Audit: solves

@onajidavid87-web Excellent work, thank you! 🎉

The PR directly fixes the critical vulnerability in issue #80 by replacing the unauthenticated set_admin() fallback branch with an explicit initialize() function requiring admin.require_auth(), and making set_admin() fail with NotInitialized when called before initialization. All code changes, tests, and deployment updates properly address the root cause. Acceptance criteria are fully met.


CI checks: ✅ PASSED: Build and Test Contracts
Merge conflicts: ✅ none

Audited by stepfi-audit-bot 🤖

@EmeditWeb
EmeditWeb merged commit b83a7bf into StepFi-app:main Aug 29, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

critical: close unauthenticated admin-claim branch in reputation set_admin()

2 participants