Skip to content

Format: fixes integer wraparound when parsing numbers #2696

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
CarterLi merged 1 commit into from
Oct 31, 2026
Merged

Format: fixes integer wraparound when parsing numbers #2696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Bugfixes:
* Improved uptime detection on SunOS via kstat. (#2674, Uptime, SunOS)
* Fixed substring formatting returning the remaining string when a negative end index is before the start of the string. (General)
* Fixed a skipped `{?}` / `{/}` conditional in a format string ending at the `{?}` / `{/}` of a conditional nested in it. (General)
* Fixed numbers in format strings that don't fit in 32 bits wrapping around to a valid argument index, constant index, substring index or length, e.g. `{4294967297}` being read as `{1}`, and a signed integer overflow for `{1:-2147483648}`. (General)

Logos:
* Added Tiny Core Linux (#2655)
Expand Down
21 changes: 14 additions & 7 deletions src/common/impl/format.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ void ffFormatAppendFormatArg(FFstrbuf* buffer, const FFformatarg* formatarg) {
}
}

// strtol() to an int32_t. Values that don't fit are clamped instead of wrapping around,
// so that e.g. 4294967297 can't turn into 1.
static int32_t strToInt32(const char* str, char** pEnd) {
long long value = strtoll(str, pEnd, 10);
return value > INT32_MAX ? INT32_MAX : value < INT32_MIN ? INT32_MIN : (int32_t) value;
}

/**
* @brief parses a string to a uint32_t
*
Expand All @@ -79,14 +86,14 @@ static uint32_t getArgumentIndex(const char* placeholderValue, uint32_t numArgs,

if (firstChar >= '0' && firstChar <= '9') {
char* pEnd = nullptr;
uint32_t result = (uint32_t) strtoul(placeholderValue, &pEnd, 10);
unsigned long result = strtoul(placeholderValue, &pEnd, 10);
if (result > numArgs) {
return UINT32_MAX;
}
if (*pEnd != '\0') {
return UINT32_MAX;
}
return result;
return (uint32_t) result;
} else if (ffCharIsEnglishAlphabet(firstChar)) {
for (uint32_t i = 0; i < numArgs; ++i) {
const FFformatarg* arg = &arguments[i];
Expand Down Expand Up @@ -525,7 +532,7 @@ static bool parseFormatString(FFstrbuf* buffer, const FFstrbuf* formatstr, uint3
// test for constant or env var, if so evaluate it
if (firstChar == '$') {
char* pend = nullptr;
int32_t indexSigned = (int32_t) strtol(placeholderValue.chars + 1, &pend, 10);
int32_t indexSigned = strToInt32(placeholderValue.chars + 1, &pend);
if (pend == placeholderValue.chars + 1) {
// treat placeholder as an environment variable
char* envValue = getenv(placeholderValue.chars + 1);
Expand Down Expand Up @@ -584,15 +591,15 @@ static bool parseFormatString(FFstrbuf* buffer, const FFstrbuf* formatstr, uint3
skipAnsiEscape(&tempString, buffer, &trailingEscape);

char* pEnd = nullptr;
int32_t start = (int32_t) strtol(pSep + 1, &pEnd, 10);
int32_t start = strToInt32(pSep + 1, &pEnd);
if (start < 0) {
start = (int32_t) tempString.length + start;
}
if (start >= 0 && (uint32_t) start < tempString.length) {
if (*pEnd == '\0') {
ffStrbufAppendNS(buffer, tempString.length - (uint32_t) start, &tempString.chars[start]);
} else if (*pEnd == ',') {
int32_t end = (int32_t) strtol(pEnd + 1, &pEnd, 10);
int32_t end = strToInt32(pEnd + 1, &pEnd);
if (!*pEnd) {
if (end < 0) {
end = (int32_t) tempString.length + end;
Expand All @@ -618,7 +625,7 @@ static bool parseFormatString(FFstrbuf* buffer, const FFstrbuf* formatstr, uint3
}
} else {
char* pEnd = nullptr;
int32_t truncLength = (int32_t) strtol(pSep + 1, &pEnd, 10);
int32_t truncLength = strToInt32(pSep + 1, &pEnd);
if (*pEnd != '\0') {
*pSep = cSep;
appendInvalidPlaceholder(buffer, "{", &placeholderValue, i, formatstr->length);
Expand All @@ -628,7 +635,7 @@ static bool parseFormatString(FFstrbuf* buffer, const FFstrbuf* formatstr, uint3
bool ellipsis = false;
if (truncLength < 0) {
ellipsis = true;
truncLength = -truncLength;
truncLength = truncLength == INT32_MIN ? INT32_MAX : -truncLength;
}

FF_STRBUF_AUTO_DESTROY tempString = ffStrbufCreate();
Expand Down
20 changes: 20 additions & 0 deletions tests/format.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,22 @@ int main(void) {
VERIFY("output({1~4})", "\e[?25l12345", "output(\e[?25l5)");
}

{
// numbers that don't fit in 32 bits must not wrap around to a valid value
VERIFY("output({4294967296})", "x", "output({4294967296})");
VERIFY("output({4294967297})", "x", "output({4294967297})");
VERIFY("output({99999999999999999999})", "x", "output({99999999999999999999})");
VERIFY("output({1~4294967297})", "12345 67890", "output()");
VERIFY("output({1~-4294967297})", "12345 67890", "output()");
VERIFY("output({1~0,4294967296})", "12345 67890", "output(12345 67890)");
VERIFY("output({1:4294967297})", "12345 67890", "output(12345 67890)");
VERIFY("output({1:-4294967296})", "12345 67890", "output(12345 67890)");

// the length is negated for the ellipsis, which overflows for the smallest int32
VERIFY("output({1:-2147483648})", "12345 67890", "output(12345 67890)");
VERIFY("output({1:-2147483647})", "12345 67890", "output(12345 67890)");
}

#ifndef _WIN32 // Windows doesn't have setenv
{
ffListInit(&instance.config.display.constants);
Expand All @@ -216,6 +232,10 @@ int main(void) {
VERIFY("output({$FF_INVAL})", "", "output({$FF_INVAL})");
VERIFY("output({$9}{$0}${-9})", "", "output({$9}{$0}${-9})");
VERIFY("output({$1NO})", "", "output({$1NO})");

// numbers that don't fit in 32 bits must not wrap around to a valid index
VERIFY("output({$4294967297})", "", "output({$4294967297})");
VERIFY("output({$-4294967297})", "", "output({$-4294967297})");
ffListDestroy(&instance.config.display.constants);
}
#endif
Expand Down
Loading