Skip to content

Harden CLI parsing and enable unsafe-buffer diagnostics#176

Closed
netliomax25-code wants to merge 1 commit into
Taywee:masterfrom
netliomax25-code:harden-cli-buffer
Closed

Harden CLI parsing and enable unsafe-buffer diagnostics#176
netliomax25-code wants to merge 1 commit into
Taywee:masterfrom
netliomax25-code:harden-cli-buffer

Conversation

@netliomax25-code
Copy link
Copy Markdown

This PR improves the security hardening posture of the project by removing raw argv range pointer arithmetic from CLI parsing paths and enabling Clang's -Wunsafe-buffer-usage diagnostics where supported.

The patch replaces direct pointer-range construction patterns such as:

args.assign(argv + 1, argv + argc);

with explicit bounds-checked iteration using standard containers.

In addition, this PR introduces regression coverage for ArgumentParser::ParseCLI(int, const char* const*) and enables compiler-assisted detection of unsafe buffer usage patterns.


Security Rationale

This change improves the auditability and safety of CLI argument parsing by:

  • Eliminating raw pointer range arithmetic in parser code paths
  • Improving compatibility with Clang Safe Buffers analysis
  • Enabling -Wunsafe-buffer-usage diagnostics to help prevent future regressions
  • Handling malformed or partially invalid argv entries more defensively
  • Making bounds reasoning explicit and easier to review

The hardening approach aligns with modern secure-by-design buffer handling practices and supports ongoing migration away from unsafe buffer manipulation idioms.


Changes

args.hxx

  • Replaced raw argv + 1, argv + argc vector assignment with explicit iteration
  • Added defensive null checks before appending CLI arguments
  • Reserved vector capacity before insertion to avoid repeated reallocations

examples/gitlike.cxx

  • Replaced raw argv pointer-range construction with explicit indexed iteration
  • Added defensive handling for null argument entries

CMakeLists.txt

  • Enabled -Wunsafe-buffer-usage when supported by Clang
  • Updated comments to reflect removal of previously incompatible raw pointer idioms

Tests

Added a regression test covering:

  • ArgumentParser::ParseCLI(int, const char* const*)

@Taywee
Copy link
Copy Markdown
Owner

Taywee commented May 27, 2026

See #173 and CONTRIBUTING.md:

  • Do not make defensive changes for conditions that will never happen. Examples:
    • Don't check for null pointers for things that can not be null, like argv[i]
      where i is in the range [0, argc). If the C standard guarantees a shape for
      argv and arc, we don't need to additionally check those ourselves.

If you want to be able to enable -Wunsafe-buffer-usage, I'm not against a change that disables that for the section, but I am against defensively protecting against something that will never happen.

@Taywee Taywee closed this May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants