Skip to content

Replace unsafe numeric parsing with std::from_chars + hardening#167

Merged
Taywee merged 2 commits into
Taywee:masterfrom
metsw24-max:numeric-parser-hardening
May 21, 2026
Merged

Replace unsafe numeric parsing with std::from_chars + hardening#167
Taywee merged 2 commits into
Taywee:masterfrom
metsw24-max:numeric-parser-hardening

Conversation

@metsw24-max

Copy link
Copy Markdown
Contributor

This pull request hardens the numeric parsing implementation in the ARGS library by replacing manual pointer-based parsing logic with the standard-library std::from_chars API, adding optional validation hardening checks, and introducing a fuzzing harness for continuous parser validation.

The previous implementation relied on custom pointer manipulation and manual base-detection logic while parsing user-controlled input. Although no direct memory corruption vulnerability was identified, these patterns increase long-term maintenance complexity and create unnecessary parsing risk around malformed, adversarial, or boundary-case numeric inputs.

This patch improves the overall security posture, reliability, and auditability of the parser while maintaining full backward compatibility.

Security Rationale

Problem Statement

The original numeric parsing implementation performed manual parsing operations on user-provided strings, including:

  • Raw pointer arithmetic
  • Manual sign handling
  • Manual numeric base detection
  • Multiple conditional parsing paths
  • Inconsistent errno restoration behavior

Example patterns from the previous implementation included:

const bool negative = begin != end_pos && *begin == '-';

if (negative || (begin != end_pos && *begin == '+')) {
    ++begin;
}

if (end_pos - begin > 1 && *begin == '0') {
    if (*(begin + 1) == 'x' || *(begin + 1) == 'X') {
        begin += 2;
        base = 16;
    }
}

###

@Taywee

Taywee commented May 19, 2026

Copy link
Copy Markdown
Owner

from_chars is a C++17 API. This would be a breaking change that would necessitate a major version bump, and I'm not comfortable committing to that at this moment.

I'm happy with a push to simplify code and improve readability, but I'd like it to remain compatible.

@Taywee Taywee left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to bump the required standard to C++17.

@metsw24-max

Copy link
Copy Markdown
Contributor Author

Thanks for the clarification.
I will rework the patch to keep compatibility with the current C++ standard and avoid introducing a C++17 requirement.
I will focus on improving readability/simplification without changing the project’s minimum standard.

@metsw24-max

Copy link
Copy Markdown
Contributor Author

@Taywee applied the changes as u requested

@Taywee

Taywee commented May 21, 2026

Copy link
Copy Markdown
Owner

Looks better. I am happy to have that hacky manual parsing implementation out of the code.

Thanks for the PR.

@Taywee Taywee merged commit 78b6348 into Taywee:master May 21, 2026
7 checks passed
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