diff --git a/src/network-services-pentesting/pentesting-web/code-review-tools.md b/src/network-services-pentesting/pentesting-web/code-review-tools.md index fe41da3ecce..d63defac560 100644 --- a/src/network-services-pentesting/pentesting-web/code-review-tools.md +++ b/src/network-services-pentesting/pentesting-web/code-review-tools.md @@ -7,6 +7,85 @@ - [**https://owasp.org/www-community/Source_Code_Analysis_Tools**](https://owasp.org/www-community/Source_Code_Analysis_Tools) - [**https://github.com/analysis-tools-dev/static-analysis**](https://github.com/analysis-tools-dev/static-analysis) +## C/C++ Manual Review Gotchas + +When reviewing **C/C++** manually, look for APIs and patterns that appear safe in isolation but become exploitable when their outputs are reused later in the control flow. + +### Non-reentrant libc return buffers breaking security checks + +Some legacy networking/string helpers return pointers to **static internal storage**. A classic example is `inet_ntoa()`: storing the returned pointer and calling the function again usually means the second call overwrites the same buffer. + +```c +char *user_ip = inet_ntoa(addr_from_user); +char *allowed_ip = inet_ntoa(addr_from_policy); + +if (strcmp(user_ip, allowed_ip) != 0) { + return DENY; +} +``` + +This kind of code can silently collapse an **allowlist / equality check** because both pointers may reference the same final string. During review, treat these APIs as suspicious whenever the returned pointer is: + +- stored for later comparison +- reused across branches +- relied on for policy decisions such as SSRF prevention or host allowlists + +Prefer APIs that write into a caller-provided buffer (`inet_ntop`, `snprintf`, explicit copies with fixed bounds). + +### Validated input reused later in `system()` / shell-outs + +A frequent review failure is validating input with a parser and later reusing the **original raw string** in a shell command: + +```c +if (!inet_aton(ip_addr, &parsed)) { + return 1; +} + +snprintf(cmd, sizeof(cmd), "ping '%s'", ip_addr); +system(cmd); +``` + +Parsing the data does **not** make the original string safe. If execution reaches `system()`, `popen()`, `execl("/bin/sh", ...)`, or similar shell-backed helpers, metacharacters in the original input can still become **command injection / RCE**. + +During review, check for this sequence: + +1. Input is parsed or normalized into a structured object. +2. A security decision is made using the parsed form. +3. The original string is later passed to a shell. + +Safer patterns: + +- avoid the shell entirely +- use `execve()`/`posix_spawn()` with a fixed argv array +- derive the executed argument from the validated canonical form instead of the original input + +### User-controlled registry/config source steering kernel control flow + +In Windows driver code, a **user-chosen registry path** or similar configuration source should not directly influence privileged control flow. Review patterns such as: + +- `WdfRequestRetrieveInputBuffer` or IOCTL input supplying a registry path +- `RtlQueryRegistryValues(..., RTL_QUERY_REGISTRY_DIRECT, ...)` writing directly into stack/local variables +- queried values selecting callbacks, operation modes, or other security-sensitive branches + +If the attacker controls the key path, they often control not only the data but also the **value type, size, and presence/absence semantics**. That can turn a "read config and choose a callback" workflow into **reliable DoS** and sometimes a **kernel code execution primitive**. + +Red flags during review: + +- absolute registry paths accepted from user mode +- no allowlist of trusted hives/keys +- no strict type/length validation before copying into integers/structs +- registry-derived values stored globally and later used as function pointers, dispatch selectors, or capability flags + +### Windows path handling footguns worth checking + +For Windows usermode reviews, explicitly audit for: + +- **unquoted path** issues in `CreateProcess*` call sites and service definitions +- ANSI/Wide-char mismatches that let Unicode characters transform during path canonicalization +- **WorstFit / Best-Fit** style issues where ANSI APIs reinterpret Unicode into separators or traversal primitives + +These are especially relevant when a path passes through validation in wide-char form but is later consumed by an ANSI API or command line builder. + ## Multi-Language Tools ### [Naxus - AI-Gents](https://www.naxusai.com/) @@ -455,7 +534,12 @@ https://github.com/securego/gosec - [https://jshint.com/](https://jshint.com/) - [https://github.com/jshint/jshint/](https://github.com/jshint/jshint/) -{{#include ../../banners/hacktricks-training.md}} +## References +- [Trail of Bits blog: Master C and C++ with our new Testing Handbook chapter](https://blog.trailofbits.com/2026/04/09/master-c-and-c-with-our-new-testing-handbook-chapter/) +- [Trail of Bits Testing Handbook: C/C++](https://appsec.guide/docs/languages/c-cpp/) +- [DEVCORE: WorstFit - Unveiling Hidden Transformers in Windows ANSI](https://devco.re/blog/2025/01/09/worstfit-unveiling-hidden-transformers-in-windows-ansi/) + +{{#include ../../banners/hacktricks-training.md}}