🔍 Security Code Review Template¶

📋 Review Metadata¶

Project/Component: _______________ Review ID: SR-YYYY-MM-DD-001 Reviewer: _______________ Review Date: _______________ Code Author: _______________ Git Commit/PR: _______________ Review Type: [ ] Pre-merge [ ] Pre-release [ ] Post-incident [ ] Routine

🎯 Focus Areas¶

1. Input Validation¶

Code Patterns to Check:

// ❌ BAD - No validation
void process(const char* input) {
    char buffer[256];
    strcpy(buffer, input);  // Buffer overflow!
}

// ✅ GOOD - Validated and safe
void process(const char* input, size_t len) {
    if (!input || len == 0 || len > MAX_INPUT_SIZE) {
        return handle_error(ERROR_INVALID_INPUT);
    }
    std::string safe_input(input, std::min(len, MAX_INPUT_SIZE));
    // ... safe processing
}

Findings:

[Document any issues found]
Line: ___
Issue: ___
Severity: ___

2. Memory Safety¶

Code Patterns to Check:

// ❌ BAD - Use after free
auto* ptr = new AudioBuffer();
delete ptr;
ptr->process();  // Use after free!

// ❌ BAD - Memory leak
void allocateBuffer() {
    auto* buf = new float[1024];
    if (error) return;  // Leak!
    delete[] buf;
}

// ✅ GOOD - Smart pointers
void processAudio() {
    auto buffer = std::make_unique<AudioBuffer>();
    buffer->process();
    // Automatic cleanup
}

Findings:

[Document any issues found]

3. Concurrency¶

No data races on shared data
Proper locking order documented and followed
Deadlock prevention (lock ordering, timeouts)
Atomic operations used correctly
Lock-free structures verified for correctness
Thread-local storage used appropriately
Audio callback is realtime-safe (no locks, no allocations)
Wait-free structures for audio thread communication
No priority inversion

Code Patterns to Check:

// ❌ BAD - Data race
class Processor {
    std::atomic<bool> running{false};
    std::vector<Sample> buffer;  // Not protected!

    void audioThread() {
        buffer.push_back(sample);  // Race!
    }
    void uiThread() {
        buffer.clear();  // Race!
    }
};

// ✅ GOOD - Protected access
class Processor {
    std::mutex mutex;
    std::vector<Sample> buffer;

    void audioThread() {
        std::lock_guard lock(mutex);
        buffer.push_back(sample);
    }
};

// ✅ BETTER - Lock-free for realtime
class Processor {
    LockFreeQueue<Sample> queue;

    void audioThread() {
        queue.push(sample);  // Wait-free
    }
};

Findings:

[Document any issues found]

4. Crypto Usage¶

Code Patterns to Check:

// ❌ BAD - Weak crypto
const char* key = "mysecretkey123";  // Hardcoded!
// Custom XOR encryption  // Don't roll your own!

// ❌ BAD - Weak algorithm
MD5_Hash(data);  // Broken algorithm
DES_Encrypt(data);  // Weak algorithm

// ✅ GOOD - Strong crypto
#include <openssl/evp.h>
EVP_EncryptInit_ex(ctx, EVP_aes_256_gcm(), NULL, key, iv);

// ✅ GOOD - Secure random
#include <openssl/rand.h>
RAND_bytes(buffer, sizeof(buffer));

Findings:

[Document any issues found]

5. Error Handling¶

Code Patterns to Check:

// ❌ BAD - Info leakage
catch (Exception& e) {
    log("Failed: " + e.message());  // May leak paths, system info
    return e.message();  // Exposed to user!
}

// ❌ BAD - Fails open
if (!authenticate(user)) {
    log("Auth failed, continuing anyway");  // Insecure!
}

// ✅ GOOD - Fails closed, no leakage
catch (Exception& e) {
    log_internal(e.details());  // Internal logging
    return "Operation failed. Contact support.";  // Generic message
}

// ✅ GOOD - Fails securely
if (!authenticate(user)) {
    log_security_event("Auth failed", user);
    return ERROR_UNAUTHORIZED;
}

Findings:

[Document any issues found]

6. Authentication & Authorization¶

Findings:

[Document any issues found]

7. Audio-Specific Security¶

Findings:

[Document any issues found]

📝 Review Notes¶

Summary¶

Brief overview of what was reviewed and overall findings.

Critical Issues¶

Issue #1:
  Location: [file:line]
  Description: [what's wrong]
  Impact: [what could happen]
  Recommendation: [how to fix]

Issue #2:
  ...

High Priority Issues¶

[Same format as critical]

Medium Priority Issues¶

[Same format as critical]

Low Priority / Recommendations¶

[Improvements that aren't security issues but would strengthen security]

Positive Observations¶

[Good security practices found in the code]

📊 Risk Assessment¶

Risk Level: [ ] Low [ ] Medium [ ] High [ ] Critical¶

Risk Calculation: - Critical Issues: ___ × 10 = ___ - High Issues: ___ × 5 = ___ - Medium Issues: ___ × 2 = ___ - Low Issues: ___ × 1 = ___ - Total Risk Score: ___

Risk Levels: - 0-5: Low Risk ✅ - 6-15: Medium Risk ⚠️ - 16-30: High Risk 🔴 - 31+: Critical Risk 🚨

✅ Approval¶

Review Decision: [ ] Approved [ ] Approved with Conditions [ ] Rejected¶

Conditions (if any):

1. Fix critical issue at line X before merge
2. Add input validation at function Y
3. Document thread safety for class Z

Blocker Issues:

[Issues that MUST be fixed before approval]

Follow-up Required:

[Issues that should be fixed but don't block this review]
Ticket #: ___

Signatures¶

Role	Name	Signature	Date
Security Reviewer	__________	__________	__________
Code Author	__________	__________	__________
Engineering Lead (if high risk)	__________	__________	__________

🔄 Follow-Up¶

Re-review Required: [ ] Yes [ ] No Re-review Date: _______________ Verification Method: [ ] Code Review [ ] Testing [ ] Automated Scan

📚 References¶

OWASP Top 10: https://owasp.org/www-project-top-ten/
CWE Top 25: https://cwe.mitre.org/top25/
CERT C++ Coding Standard: https://wiki.sei.cmu.edu/confluence/x/fXZGBQ
Audio Thread Safety: [Internal docs]

📋 Reviewer Checklist¶

Before Review¶

Code compiles without warnings
Static analysis passed
Tests passing
Review scope clear

During Review¶

After Review¶

Review Template Version: 1.0 Last Updated: [Date] Owner: Security Team