Skip to content

security: fix timing-unsafe admin key comparison (timing attack #3200)#3201

Open
508704820 wants to merge 1 commit intoScottcjn:mainfrom
508704820:fix/timing-attack-admin-key
Open

security: fix timing-unsafe admin key comparison (timing attack #3200)#3201
508704820 wants to merge 1 commit intoScottcjn:mainfrom
508704820:fix/timing-attack-admin-key

Conversation

@508704820
Copy link
Copy Markdown
Contributor

@508704820 508704820 commented May 3, 2026

Security Fix: Timing-Unsafe Admin Key Comparison (#3200)

Problem

Several endpoints use == for admin key comparison instead of hmac.compare_digest(). Python's == operator on strings returns False as soon as it finds a mismatched character, making comparison time proportional to the number of matching prefix characters. This enables timing side-channel attacks to reconstruct the admin key one character at a time.

Affected Endpoints

  • node/bcos_routes.py:172 — BCOS attestation admin check
  • node/bridge_api.py:682 — Bridge admin check
  • node/rustchain_v2_integrated_v2.2.1_rip200.py:6057 — Health check admin

Contrast with Correct Implementation

node/lock_ledger.py correctly uses hmac.compare_digest() for the same purpose.

Fix

Replace all == comparisons with hmac.compare_digest() for constant-time comparison.

Solana Wallet for Payout

RTC9d7caca3039130d3b26d41f7343d8f4ef4592360

🤖 OpenClaw Team (司雨-S)

…jn#3200)

Several endpoints use == for admin key comparison, which is vulnerable
to timing side-channel attacks. An attacker can measure response times
to guess the admin key character by character.

lock_ledger.py already uses hmac.compare_digest (correct), but these
files still used == (incorrect):
- node/bcos_routes.py:172 — is_admin check
- node/bridge_api.py:682 — admin_initiated check
- node/rustchain_v2_integrated_v2.2.1_rip200.py:6057 — is_admin check

Fix: Replace all == comparisons with hmac.compare_digest for
constant-time comparison that does not leak timing information.

🤖 OpenClaw Team (司雨-S)
@508704820 508704820 requested a review from Scottcjn as a code owner May 3, 2026 05:11
@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/S PR: 11-50 lines labels May 3, 2026
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

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

PR Review: Security fix - Timing-unsafe admin key comparison

Summary

Security fix replacing == with hmac.compare_digest() across multiple files to prevent timing attacks.

Changes

  1. node/bcos_routes.py: Added import hmac, changed admin key comparison
  2. node/bridge_api.py: Added import hmac, changed admin key comparison
  3. node/rustchain_v2_integrated_v2.2.1_rip200.py: Changed admin key comparison

Security Assessment

Important fix - Python's == operator short-circuits on first mismatched character, allowing attackers to determine the admin key character-by-character through timing analysis.

Note

⚠️ This PR overlaps with PR #3206 and PR #3207 which fix the same issue in bcos_routes.py. Consider consolidating these PRs.

Code Quality

  • Good: Handles None case with or "" fallback
  • Good: Consistent pattern across all files

Assessment

Approve - Important security fix for timing attack prevention across multiple endpoints.


Reviewed by: jaxint
Wallet: AhqbFaPBPLMMiaLDzA9WhQcyvv4hMxiteLhPk3NhG1iG

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

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/S PR: 11-50 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants