Start a C++ security review at the trust boundaries: every point where the program accepts files, network data, command-line arguments, environment variables, or input from another process. A value that looks harmless at the interface may later become a size, array index, pointer offset, file path, or format string. Following that value through the code is often more useful than reading the source strictly from top to bottom.
Secure code review is a deliberate examination of source code and its assumptions to identify conditions that could affect confidentiality, integrity, availability, or safe program behavior. It complements compilation, automated tests, and static analysis. None of those checks can prove that an application handles hostile or malformed input safely.
Prepare the review before reading the diff
Begin with the scope of the change. Identify what changed, which components consume it, and whether it crosses a security boundary. A small patch can have broad effects when it changes parsing, authentication decisions, memory ownership, serialization, logging, or file operations that depend on privileges.
- Read the change description and linked requirements.
- Build the project with its supported compiler and warning settings.
- Run existing unit and integration tests in an authorized development environment.
- Identify externally controlled inputs and the assets they may affect.
- Check project conventions for ownership, error handling, threading, and supported C++ versions.
Useful review comments are specific and testable. Instead of writing “this looks unsafe,” describe the data flow and the consequence: “payload_length comes from the packet header and is converted to size_t before the maximum is checked; a negative signed value can become a large allocation request.”
Follow data from entry point to dangerous operation
For each untrusted value, find the validation step, then confirm that the validated value is the one later used. Validation can be undermined by casts, arithmetic, copies into smaller types, or later modifications. Pay close attention to values used for memory allocation, array access, byte copying, file-path construction, subprocess calls, or authorization decisions.
Check input validation and numeric conversions
C++ makes it easy to mix signed and unsigned values, which often leads to bounds-checking errors. A negative int converted to size_t becomes a very large positive number. Arithmetic can also overflow before the code reaches a capacity check.
Set clear invariants before performing calculations. If a buffer needs count * element_size bytes, verify that count is within a documented maximum and that the multiplication cannot exceed either the destination capacity or the largest representable size. A length is not automatically safe just because it fits the input type.
| Review target | Questions to ask |
|---|---|
| Array or vector access | Is the index checked against the current container size before access? |
| Length field | Is it validated before allocation, copying, looping, or casting? |
| Integer arithmetic | Can addition, multiplication, subtraction, or conversion overflow? |
| Parser state | Does every read confirm enough input remains? |
| Path input | Is the resolved path restricted to an approved location? |
Examine memory ownership and object lifetime
Memory defects remain a central concern in C++ security reviews. Look for raw owning pointers, manual new/delete, pointer arithmetic, C-style arrays, and functions that return references or views. These are not automatically unsafe, but each needs a clear ownership model.
Ask who owns every resource, when ownership changes, and whether an object can be used after it is destroyed. A pointer or reference may become invalid when a std::vector reallocates, a temporary object expires, or a callback outlives the data it captured. Returned std::string_view and std::span need particular scrutiny because they do not own their underlying storage.
When a design change is practical, prefer RAII-managed resources and standard containers. std::unique_ptr communicates exclusive ownership. Use std::shared_ptr only when the lifetime is genuinely shared, since shared ownership can hide cycles and make cleanup order harder to understand.
Review APIs, errors, and unsafe interfaces
Flag legacy interfaces that cannot express destination capacity, including unbounded string-copying functions. Inspect formatting calls as well: externally supplied data must never become the format string. Safer interfaces still require care. A bounded copy that silently truncates an authentication token or configuration value can introduce a different security problem.
Error paths matter as much as successful ones. Check that failures stop sensitive operations, release resources through RAII, and do not continue with default-initialized or partially parsed data. Logs should give operators enough context to diagnose a failure without exposing secrets, access tokens, private keys, or complete personal data.
Do not overlook concurrency
When code uses threads, callbacks, or asynchronous tasks, inspect shared state outside the happy path. A mutex may protect one read while another write remains unprotected. Look for time-of-check/time-of-use gaps, particularly around files, permissions, and mutable configuration. A validation result is useful only when the relevant object cannot change before the protected action takes place.
Use tools as evidence, not as approval stamps
Build with strong compiler warnings and treat new warnings as review items. Static analyzers can highlight suspicious null handling, unchecked return values, unreachable branches, and possible lifetime issues. AddressSanitizer and UndefinedBehaviorSanitizer are especially useful for tests involving parsers and memory-heavy code in a controlled test environment.
Tools have limits. They can miss flaws hidden by incomplete test coverage, project-specific assumptions, or business logic, and their findings still require human judgment. Review the code even when automated checks pass. If a warning appears, investigate it rather than suppressing it just to keep the pipeline green.
Record findings and verify the fix
Classify each finding by impact, exploitability in the application’s intended environment, and confidence. A good report names the affected file and line, describes the unsafe condition, explains a realistic consequence, and suggests a direction for the fix. Keep the discussion about the code and its behavior, not the author.
After the fix, trace the revised data flow instead of confirming only that the original line changed. Add a regression test for the boundary condition. For example, a parser given a declared length larger than the remaining bytes should return a controlled error before allocating memory or reading data. That test helps preserve the same security property as the C++ code changes over time.
