If a trailing `%' character occurs, we read past the end of the format
string and thus introduce UB. Reproducible by executing the following:
./printf %
This happens because the format string here actually consists of two
characters, `%' _and_ the trailing nul-byte. The flag parsing loop
matches the nul-byte with `0' and thus increases the counter, `i', to
be `formatlen + 1'. Furthermore, `i' is used as an index to access the
format string, which will eventually lead to an out-of-bounds access.
This can be fixed by simply checking the value of `i' like this:
if (i > formatlen) eprintf(...);
However, there are two more ways in which `i' can "overflow".
The second "overflow" could happen after parsing the "field width".
Given the following call,
./printf %42
we enter the loop with `i = 1'. The condition is matched _3_ times, again,
because of the trailing nul-byte. `i' now has a value of `formatlen + 1'.
This can be fixed by using the same check as above.
The last way in which `i' might "overflow" is after parsing the "field precision".
Take this call as an example:
./printf %.42
Here, we enter the loop with `i = formatlen'. After the dot (`.') is matched,
`i' is increased by one and effectively "overflows".
The fix for this is the same as above. I agree that it's a bit ugly to
repeat this check three times, but the alternatives seem to add more cruft.
Note that the last two cases only appear if numbers are used after the
percent sign. Other characters don't match the conditions and just
fall through to the `switch' statements default case, which emits an error.
---
Although I've tested this, it's not unlikely that I've overseen something.
As stated above, this solution isn't beautiful, but after playing around
a bit with different approaches, it turned out to be the most convenient
(I'm happy to be corrected on this). Anyways, it's definitely better than
having UB in a tool which is marked as "finished".
---
printf.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/printf.c b/printf.c
index 039dac717105..75667dad8d36 100644
--- a/printf.c
+++ b/printf.c
_AT_@ -54,6 +54,9 @@ main(int argc, char *argv[])
flag = format[i];
}
+ if (i > formatlen)
+ eprintf("Missing conversion specifier.\n");
+
/* field width */
width = -1;
if (format[i] == '*') {
_AT_@ -74,6 +77,9 @@ main(int argc, char *argv[])
}
}
+ if (i > formatlen)
+ eprintf("Missing conversion specifier.\n");
+
/* field precision */
precision = -1;
if (format[i] == '.') {
_AT_@ -96,6 +102,9 @@ main(int argc, char *argv[])
}
}
+ if (i > formatlen)
+ eprintf("Missing conversion specifier.\n");
+
if (format[i] != '%') {
if (argi < argc)
arg = argv[argi++];
--
2.37.2
Received on Mon Aug 29 2022 - 22:49:09 CEST