Skip to content

🛡️ Sentinel: [HIGH] Fix TOCTOU race condition in signature verifier nonce checking - #128

Merged
blue4209211 merged 1 commit into
mainfrom
fix-nonce-toctou-race
Aug 8, 2026
Merged

🛡️ Sentinel: [HIGH] Fix TOCTOU race condition in signature verifier nonce checking#128
blue4209211 merged 1 commit into
mainfrom
fix-nonce-toctou-race

Conversation

@blue4209211

@blue4209211 blue4209211 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Description

  • 🚨 Severity: HIGH (Replay Protection Bypass)
  • 💡 Vulnerability: In pkg/signing/verify.go, nonces were checked with isReplayedNonce() prior to signature verification, and recorded with recordNonce() only at the end of Verify().
  • 🎯 Impact: Under concurrent load, multiple identical signed requests could execute isReplayedNonce() simultaneously before any goroutine recorded the nonce, allowing attackers to bypass replay attack protection and execute duplicate signed commands on datasources.
  • 🔧 Fix:
    1. Updated pkg/signing/verify.go to perform cryptographic Ed25519 signature and payload integrity verification first.
    2. Replaced separate isReplayedNonce and recordNonce calls with an atomic checkAndRecordNonce(nonce) method under mutex lock v.nonceMu.
    3. Added TestVerify_ConcurrentReplay in pkg/signing/verify_test.go to test concurrent replay protection under 20 concurrent goroutines.
    4. Added critical security learning entry to .jules/sentinel.md.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Enhancement (non-breaking change which improves existing functionality)
  • Refactor (non-breaking change which improves code structure)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation
  • CI/CD

How Has This Been Tested?

  • Unit tests
  • Manual testing

Checklist

  • Commits are signed off (git commit -s) per DCO
  • make validate passes (fmt + lint + test)
  • Docs updated if the wire shape, config surface, or proxy module behavior changed

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request resolves a TOCTOU race condition in the signature verification process by combining the nonce check and record operations into a single atomic method, and adds a concurrent replay test to verify the fix. The review feedback identifies a performance bottleneck in the nonce eviction logic under high concurrent load and suggests rate-limiting the eviction process to prevent latency spikes.

Comment thread pkg/signing/verify.go
@blue4209211
blue4209211 force-pushed the fix-nonce-toctou-race branch from 61fa67c to 25c874e Compare August 8, 2026 05:48
…once checking

Signed-off-by: shiv <blue.4209211@gmail.com>
@mayankpande88
mayankpande88 force-pushed the fix-nonce-toctou-race branch from 25c874e to c433a14 Compare August 8, 2026 05:56
@blue4209211
blue4209211 merged commit aa512b6 into main Aug 8, 2026
6 checks passed
@blue4209211
blue4209211 deleted the fix-nonce-toctou-race branch August 8, 2026 06:06
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.

2 participants