|
7 | 7 | - [**https://owasp.org/www-community/Source_Code_Analysis_Tools**](https://owasp.org/www-community/Source_Code_Analysis_Tools) |
8 | 8 | - [**https://github.com/analysis-tools-dev/static-analysis**](https://github.com/analysis-tools-dev/static-analysis) |
9 | 9 |
|
| 10 | +## C/C++ Manual Review Gotchas |
| 11 | + |
| 12 | +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. |
| 13 | + |
| 14 | +### Non-reentrant libc return buffers breaking security checks |
| 15 | + |
| 16 | +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. |
| 17 | + |
| 18 | +```c |
| 19 | +char *user_ip = inet_ntoa(addr_from_user); |
| 20 | +char *allowed_ip = inet_ntoa(addr_from_policy); |
| 21 | + |
| 22 | +if (strcmp(user_ip, allowed_ip) != 0) { |
| 23 | + return DENY; |
| 24 | +} |
| 25 | +``` |
| 26 | + |
| 27 | +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: |
| 28 | + |
| 29 | +- stored for later comparison |
| 30 | +- reused across branches |
| 31 | +- relied on for policy decisions such as SSRF prevention or host allowlists |
| 32 | + |
| 33 | +Prefer APIs that write into a caller-provided buffer (`inet_ntop`, `snprintf`, explicit copies with fixed bounds). |
| 34 | + |
| 35 | +### Validated input reused later in `system()` / shell-outs |
| 36 | + |
| 37 | +A frequent review failure is validating input with a parser and later reusing the **original raw string** in a shell command: |
| 38 | + |
| 39 | +```c |
| 40 | +if (!inet_aton(ip_addr, &parsed)) { |
| 41 | + return 1; |
| 42 | +} |
| 43 | + |
| 44 | +snprintf(cmd, sizeof(cmd), "ping '%s'", ip_addr); |
| 45 | +system(cmd); |
| 46 | +``` |
| 47 | +
|
| 48 | +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**. |
| 49 | +
|
| 50 | +During review, check for this sequence: |
| 51 | +
|
| 52 | +1. Input is parsed or normalized into a structured object. |
| 53 | +2. A security decision is made using the parsed form. |
| 54 | +3. The original string is later passed to a shell. |
| 55 | +
|
| 56 | +Safer patterns: |
| 57 | +
|
| 58 | +- avoid the shell entirely |
| 59 | +- use `execve()`/`posix_spawn()` with a fixed argv array |
| 60 | +- derive the executed argument from the validated canonical form instead of the original input |
| 61 | +
|
| 62 | +### User-controlled registry/config source steering kernel control flow |
| 63 | +
|
| 64 | +In Windows driver code, a **user-chosen registry path** or similar configuration source should not directly influence privileged control flow. Review patterns such as: |
| 65 | +
|
| 66 | +- `WdfRequestRetrieveInputBuffer` or IOCTL input supplying a registry path |
| 67 | +- `RtlQueryRegistryValues(..., RTL_QUERY_REGISTRY_DIRECT, ...)` writing directly into stack/local variables |
| 68 | +- queried values selecting callbacks, operation modes, or other security-sensitive branches |
| 69 | +
|
| 70 | +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**. |
| 71 | +
|
| 72 | +Red flags during review: |
| 73 | +
|
| 74 | +- absolute registry paths accepted from user mode |
| 75 | +- no allowlist of trusted hives/keys |
| 76 | +- no strict type/length validation before copying into integers/structs |
| 77 | +- registry-derived values stored globally and later used as function pointers, dispatch selectors, or capability flags |
| 78 | +
|
| 79 | +### Windows path handling footguns worth checking |
| 80 | +
|
| 81 | +For Windows usermode reviews, explicitly audit for: |
| 82 | +
|
| 83 | +- **unquoted path** issues in `CreateProcess*` call sites and service definitions |
| 84 | +- ANSI/Wide-char mismatches that let Unicode characters transform during path canonicalization |
| 85 | +- **WorstFit / Best-Fit** style issues where ANSI APIs reinterpret Unicode into separators or traversal primitives |
| 86 | +
|
| 87 | +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. |
| 88 | +
|
10 | 89 | ## Multi-Language Tools |
11 | 90 |
|
12 | 91 | ### [Naxus - AI-Gents](https://www.naxusai.com/) |
@@ -455,7 +534,12 @@ https://github.com/securego/gosec |
455 | 534 | - [https://jshint.com/](https://jshint.com/) |
456 | 535 | - [https://github.com/jshint/jshint/](https://github.com/jshint/jshint/) |
457 | 536 |
|
458 | | -{{#include ../../banners/hacktricks-training.md}} |
| 537 | +## References |
459 | 538 |
|
| 539 | +- [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/) |
| 540 | +- [Trail of Bits Testing Handbook: C/C++](https://appsec.guide/docs/languages/c-cpp/) |
| 541 | +- [DEVCORE: WorstFit - Unveiling Hidden Transformers in Windows ANSI](https://devco.re/blog/2025/01/09/worstfit-unveiling-hidden-transformers-in-windows-ansi/) |
| 542 | + |
| 543 | +{{#include ../../banners/hacktricks-training.md}} |
460 | 544 |
|
461 | 545 |
|
0 commit comments