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.
This commit is contained in:
arxhr007
2026-10-31 15:08:52 +05:30
committed by Carter Li
parent d4dd15763f
commit d6c15e1cec
3 changed files with 35 additions and 7 deletions
+1
View File
@@ -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)
+14 -7
View File
@@ -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();
+20
View File
@@ -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