From d6c15e1cecfd0f7fa8dc7f0c032cda00d6cb221b Mon Sep 17 00:00:00 2001 From: arxhr007 Date: Sat, 31 Oct 2026 15:08:52 +0530 Subject: [PATCH] Format: fixes integer wraparound when parsing numbers Numbers that don't fit in 32 bits were truncated, so `{4294967297}` was read as `{1}`, `{4294967296}` as the next argument, and `{$4294967297}` as the first constant. The same applied to the start, end and length of `~`, `:`, `<`, `>` and `|`. Clamp them instead, so that they stay out of range. Also avoid negating INT32_MIN for the ellipsis length of `{1:-2147483648}`, which is a signed integer overflow. --- CHANGELOG.md | 1 + src/common/impl/format.c | 21 ++++++++++++++------- tests/format.c | 20 ++++++++++++++++++++ 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 238ba598a..f32a63369 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/src/common/impl/format.c b/src/common/impl/format.c index 48eb4321c..3fdae18e6 100644 --- a/src/common/impl/format.c +++ b/src/common/impl/format.c @@ -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 * @@ -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]; @@ -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); @@ -584,7 +591,7 @@ 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; } @@ -592,7 +599,7 @@ static bool parseFormatString(FFstrbuf* buffer, const FFstrbuf* formatstr, uint3 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; @@ -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); @@ -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(); diff --git a/tests/format.c b/tests/format.c index 17fb50add..223054075 100644 --- a/tests/format.c +++ b/tests/format.c @@ -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); @@ -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