Skip to content

<xlocnum>: Defend against sprintf_s() failure in num_put::do_put() #5925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
StephanTLavavej merged 2 commits into from
Dec 2, 2025
Merged

<xlocnum>: Defend against sprintf_s() failure in num_put::do_put() #5925

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16fd277
Extract sprintf_s(), defend against negative values.
StephanTLavavej Dec 1, 2025
46dfbdc
Extract `_Ifmt()` and `_Ffmt()` to debulk the `sprintf_s()` calls.
StephanTLavavej Dec 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 74 additions & 20 deletions stl/inc/xlocnum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1313,35 +1313,67 @@ protected:
char _Buf[2 * _Max_int_dig];
char _Fmt[6];

return _Iput(_Dest, _Iosbase, _Fill, _Buf,
static_cast<size_t>(_CSTD sprintf_s(_Buf, sizeof(_Buf), _Ifmt(_Fmt, "ld", _Iosbase.flags()), _Val)));
(void) _Ifmt(_Fmt, "ld", _Iosbase.flags());

const int _Count = _CSTD sprintf_s(_Buf, sizeof(_Buf), _Fmt, _Val);

if (_Count < 0) { // It should be impossible for sprintf_s() to fail here.
_STL_INTERNAL_CHECK(false); // In STL tests, report an error.
return _Dest; // In production, return early to avoid static_cast<size_t> of a negative _Count.
}

return _Iput(_Dest, _Iosbase, _Fill, _Buf, static_cast<size_t>(_Count));
}

virtual _OutIt __CLR_OR_THIS_CALL do_put(_OutIt _Dest, ios_base& _Iosbase, _Elem _Fill,
unsigned long _Val) const { // put formatted unsigned long to _Dest
char _Buf[2 * _Max_int_dig];
char _Fmt[6];

return _Iput(_Dest, _Iosbase, _Fill, _Buf,
static_cast<size_t>(_CSTD sprintf_s(_Buf, sizeof(_Buf), _Ifmt(_Fmt, "lu", _Iosbase.flags()), _Val)));
(void) _Ifmt(_Fmt, "lu", _Iosbase.flags());

const int _Count = _CSTD sprintf_s(_Buf, sizeof(_Buf), _Fmt, _Val);

if (_Count < 0) { // It should be impossible for sprintf_s() to fail here.
_STL_INTERNAL_CHECK(false); // In STL tests, report an error.
return _Dest; // In production, return early to avoid static_cast<size_t> of a negative _Count.
}
Comment on lines +1337 to +1340
Copy link
Member

@davidmrdavid davidmrdavid Dec 1, 2025

Choose a reason for hiding this comment

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

It's a non-blocking concern, but my instinct would be to fastfail (like in hardening) in production if any 'impossible' intermediate results are detected. Other than for ease of implementation, is there a reason not to do that?

My instinct is to avoid encoding silent failure at all costs, so I'd prefer not to have this 'impossible' path in the first place. But I'll trust your intuition here on whether this is a reasonable concern.

Copy link
Member Author

@StephanTLavavej StephanTLavavej Dec 1, 2025

Choose a reason for hiding this comment

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

I'll think about that for a potential followup. My main concern would be whatever static analysis tool was involved here not being able to detect a noreturn codepath.


return _Iput(_Dest, _Iosbase, _Fill, _Buf, static_cast<size_t>(_Count));
}

virtual _OutIt __CLR_OR_THIS_CALL do_put(
_OutIt _Dest, ios_base& _Iosbase, _Elem _Fill, long long _Val) const { // put formatted long long to _Dest
char _Buf[2 * _Max_int_dig];
char _Fmt[8];

return _Iput(_Dest, _Iosbase, _Fill, _Buf,
static_cast<size_t>(_CSTD sprintf_s(_Buf, sizeof(_Buf), _Ifmt(_Fmt, "Ld", _Iosbase.flags()), _Val)));
(void) _Ifmt(_Fmt, "Ld", _Iosbase.flags());

const int _Count = _CSTD sprintf_s(_Buf, sizeof(_Buf), _Fmt, _Val);

if (_Count < 0) { // It should be impossible for sprintf_s() to fail here.
_STL_INTERNAL_CHECK(false); // In STL tests, report an error.
return _Dest; // In production, return early to avoid static_cast<size_t> of a negative _Count.
}

return _Iput(_Dest, _Iosbase, _Fill, _Buf, static_cast<size_t>(_Count));
}

virtual _OutIt __CLR_OR_THIS_CALL do_put(_OutIt _Dest, ios_base& _Iosbase, _Elem _Fill,
unsigned long long _Val) const { // put formatted unsigned long long to _Dest
char _Buf[2 * _Max_int_dig];
char _Fmt[8];

return _Iput(_Dest, _Iosbase, _Fill, _Buf,
static_cast<size_t>(_CSTD sprintf_s(_Buf, sizeof(_Buf), _Ifmt(_Fmt, "Lu", _Iosbase.flags()), _Val)));
(void) _Ifmt(_Fmt, "Lu", _Iosbase.flags());

const int _Count = _CSTD sprintf_s(_Buf, sizeof(_Buf), _Fmt, _Val);

if (_Count < 0) { // It should be impossible for sprintf_s() to fail here.
_STL_INTERNAL_CHECK(false); // In STL tests, report an error.
return _Dest; // In production, return early to avoid static_cast<size_t> of a negative _Count.
}

return _Iput(_Dest, _Iosbase, _Fill, _Buf, static_cast<size_t>(_Count));
}

virtual _OutIt __CLR_OR_THIS_CALL do_put(
Expand All @@ -1365,10 +1397,17 @@ protected:
const bool _Is_finite = (_STD isfinite) (_Val);
const auto _Adjusted_flags = // TRANSITION, DevCom-10519861
_Is_finite ? _Iosbase.flags() : _Iosbase.flags() & ~ios_base::showpoint;
const auto _Ngen = static_cast<size_t>(_CSTD sprintf_s(
&_Buf[0], _Buf.size(), _Ffmt(_Fmt, 0, _Adjusted_flags), static_cast<int>(_Precision), _Val));

return _Fput_v3(_Dest, _Iosbase, _Fill, _Buf.c_str(), _Ngen, _Is_finite);
(void) _Ffmt(_Fmt, 0, _Adjusted_flags);

const int _Count = _CSTD sprintf_s(&_Buf[0], _Buf.size(), _Fmt, static_cast<int>(_Precision), _Val);

if (_Count < 0) { // It should be impossible for sprintf_s() to fail here.
_STL_INTERNAL_CHECK(false); // In STL tests, report an error.
return _Dest; // In production, return early to avoid static_cast<size_t> of a negative _Count.
}

return _Fput_v3(_Dest, _Iosbase, _Fill, _Buf.c_str(), static_cast<size_t>(_Count), _Is_finite);
}

virtual _OutIt __CLR_OR_THIS_CALL do_put(
Expand All @@ -1392,24 +1431,38 @@ protected:
const bool _Is_finite = (_STD isfinite) (_Val);
const auto _Adjusted_flags = // TRANSITION, DevCom-10519861
_Is_finite ? _Iosbase.flags() : _Iosbase.flags() & ~ios_base::showpoint;
const auto _Ngen = static_cast<size_t>(_CSTD sprintf_s(
&_Buf[0], _Buf.size(), _Ffmt(_Fmt, 'L', _Adjusted_flags), static_cast<int>(_Precision), _Val));

return _Fput_v3(_Dest, _Iosbase, _Fill, _Buf.c_str(), _Ngen, _Is_finite);
(void) _Ffmt(_Fmt, 'L', _Adjusted_flags);

const int _Count = _CSTD sprintf_s(&_Buf[0], _Buf.size(), _Fmt, static_cast<int>(_Precision), _Val);

if (_Count < 0) { // It should be impossible for sprintf_s() to fail here.
_STL_INTERNAL_CHECK(false); // In STL tests, report an error.
return _Dest; // In production, return early to avoid static_cast<size_t> of a negative _Count.
}

return _Fput_v3(_Dest, _Iosbase, _Fill, _Buf.c_str(), static_cast<size_t>(_Count), _Is_finite);
}
#pragma warning(pop)

virtual _OutIt __CLR_OR_THIS_CALL do_put(
_OutIt _Dest, ios_base& _Iosbase, _Elem _Fill, const void* _Val) const { // put formatted void pointer to _Dest
char _Buf[2 * _Max_int_dig];

return _Iput(
_Dest, _Iosbase, _Fill, _Buf, static_cast<size_t>(_CSTD sprintf_s(_Buf, sizeof(_Buf), "%p", _Val)));
const int _Count = _CSTD sprintf_s(_Buf, sizeof(_Buf), "%p", _Val);

if (_Count < 0) { // It should be impossible for sprintf_s() to fail here.
_STL_INTERNAL_CHECK(false); // In STL tests, report an error.
return _Dest; // In production, return early to avoid static_cast<size_t> of a negative _Count.
}

return _Iput(_Dest, _Iosbase, _Fill, _Buf, static_cast<size_t>(_Count));
}

private:
char* __CLRCALL_OR_CDECL _Ffmt(
char* _Fmt, char _Spec, ios_base::fmtflags _Flags) const { // generate sprintf format for floating-point
// TRANSITION, ABI: This always returns _Fmt, which is now ignored.
char* __CLRCALL_OR_CDECL _Ffmt(char* _Fmt, char _Spec, ios_base::fmtflags _Flags) const {
// generate sprintf format for floating-point
char* _Ptr = _Fmt;
*_Ptr++ = '%';

Expand Down Expand Up @@ -1531,8 +1584,9 @@ private:
return _Rep(_Dest, _Fill, _Fillcount); // put trailing fill
}

char* __CLRCALL_OR_CDECL _Ifmt(
char* _Fmt, const char* _Spec, ios_base::fmtflags _Flags) const { // generate sprintf format for integer
// TRANSITION, ABI: This always returns _Fmt, which is now ignored.
char* __CLRCALL_OR_CDECL _Ifmt(char* _Fmt, const char* _Spec, ios_base::fmtflags _Flags) const {
// generate sprintf format for integer
char* _Ptr = _Fmt;
*_Ptr++ = '%';

Expand Down