security-review

name	security-review
description	Review Dart FFI code for security issues in cryptographic contexts. Use when reviewing code changes, checking for memory leaks, verifying secure memory handling, or auditing cryptographic code. Use when this capability is needed.
metadata	{"author":"djx-y-z"}

Security Review for liboqs_dart

Review code for security issues specific to this post-quantum cryptography FFI library.

Security Checklist

1. Memory Management

Secret Data (keys, shared secrets):

• Use LibOQSUtils.secureFreePointer(ptr, length) - calls OQS_MEM_secure_free
• Never use LibOQSUtils.freePointer() for secrets

Non-sensitive Data (public keys, ciphertext):

• Use LibOQSUtils.freePointer(ptr) - standard free is OK

Always:

• Free memory in finally blocks to prevent leaks on exceptions

2. Classes Holding Secrets

Every class with secret data MUST have:

// 1. Finalizer for automatic cleanup (defense-in-depth)
final Finalizer<Uint8List> _secretDataFinalizer = Finalizer((data) {
  LibOQSUtils.zeroMemory(data);
});

// 2. In constructor - attach finalizer
_secretDataFinalizer.attach(this, secretKey, detach: this);

// 3. clearSecrets() method for explicit cleanup
void clearSecrets() {
  LibOQSUtils.zeroMemory(secretKey);
}

3. Comparisons

• Use LibOQSUtils.constantTimeEquals() for comparing secrets
• NEVER use == or loop-based comparison for secret data (timing attacks!)

4. Function Pointers

Before calling .asFunction() on native function pointers:

// CORRECT: Validate first
if (_ptr.ref.keypair == nullptr) {
  throw LibOQSException('keypair function pointer is null - corrupted');
}
final fn = _ptr.ref.keypair.asFunction<...>();

// WRONG: Direct call without validation
final fn = _ptr.ref.keypair.asFunction<...>(); // May crash!

5. dispose() Pattern

Order matters - prevents memory leaks if exception occurs:

void dispose() {
  if (!_disposed) {
    oqs.OQS_KEM_free(_kemPtr);      // 1. Free native memory
    _kemFinalizer.detach(this);      // 2. Detach finalizer
    _disposed = true;                 // 3. Set flag
  }
}

6. Documentation

Methods exposing secrets MUST have:

/// **Security Warning:** This method exports the SECRET KEY in plaintext.
/// Only use for secure storage. Never log the output.

7. What to Look For

Red Flags:

• freePointer used on secret keys or shared secrets
• Missing clearSecrets() method on classes with secrets
• Missing Finalizer attachment for secret data
• == used to compare keys or secrets
• Function pointer used without null check
• Missing finally block for memory cleanup
• Logging or printing of secret data

Good Patterns:

• secureFreePointer for all secrets
• constantTimeEquals for secret comparison
• Finalizers attached in constructors
• clearSecrets() available and documented
• Security warnings in docstrings

Example Review Output

## Security Review: lib/src/new_feature.dart

### Issues Found

1. **Line 45**: Using `freePointer` for secret key
   - Severity: HIGH
   - Fix: Use `secureFreePointer(secretKeyPtr, secretKey.length)`

2. **Line 78**: Missing Finalizer for shared secret
   - Severity: MEDIUM
   - Fix: Add `_secretDataFinalizer.attach(this, sharedSecret, detach: this)`

### Recommendations

- Add `clearSecrets()` method to `NewResult` class
- Add security warning to `toStrings()` docstring

---

Converted and distributed by TomeVault — claim your Tome and manage your conversions.