eyalroz
Possible NULL pointer dereference
Hi,
When I build printf with GCC 12.2 from ARM I get the following error
FAILED: E70_Bootloader/CMakeFiles/E70_Bootloader.dir/__/printf/src/printf/printf.c.obj
@arm-gnu-toolchain-12.2.rel1-mingw-w64-i686-arm-none-eabi\bin\arm-none-eabi-gcc.exe -DSAME70=1 -D__SAME70Q21B__ -IE70_Boot_loader/E70_Bootloader/. -IE70_Boot_loader/E70_Bootloader/../printf/src -g3 -Og -Wall -pedantic -DDEBUG -mthumb -mcpu=cortex-m7 -mfloat-abi=hard -mfpu=fpv5-d16 -specs=nano.specs -fcallgraph-info=su,da -fstack-protector-strong -mno-unaligned-access -Wall -Wextra -Wpedantic -Werror -Wformat=2 -Wcast-align -Wsign-conversion -Wdouble-promotion -Wshadow -Wstrict-prototypes -Wold-style-definition -Wpadded -Wlogical-op -fanalyzer -fno-builtin-printf -Wno-padded -MD -MT E70_Bootloader/CMakeFiles/E70_Bootloader.dir/__/printf/src/printf/printf.c.obj -MF E70_Bootloader\CMakeFiles\E70_Bootloader.dir\__\printf\src\printf\printf.c.obj.d -o E70_Bootloader/CMakeFiles/E70_Bootloader.dir/__/printf/src/printf/printf.c.obj -c E70_Boot_loader/printf/src/printf/printf.c
E70_Boot_loader/printf/src/printf/printf.c: In function 'putchar_via_gadget':
E70_Boot_loader\printf\src\printf\printf.c(360,31): error GAD6F4944: dereference of NULL '0' [CWE-476] [-Werror=analyzer-null-dereference]
360 | gadget->buffer[write_pos] = c;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
'fctprintf': events 1-2
|
| 1452 | int fctprintf(void (*out)(char c, void* extra_arg), void* extra_arg, const char* format, ...)
| | ^~~~~~~~~
| | |
| | (1) entry to 'fctprintf'
|......
| 1456 | const int ret = vfctprintf(out, extra_arg, format, args);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (2) calling 'vfctprintf' from 'fctprintf'
|
+--> 'vfctprintf': events 3-4
|
| 1419 | int vfctprintf(void (*out)(char c, void* extra_arg), void* extra_arg, const char* format, va_list arg)
| | ^~~~~~~~~~
| | |
| | (3) entry to 'vfctprintf'
| 1420 | {
| 1421 | output_gadget_t gadget = function_gadget(out, extra_arg);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (4) calling 'function_gadget' from 'vfctprintf'
|
+--> 'function_gadget': events 5-6
|
| 408 | static inline output_gadget_t function_gadget(void (*function)(char, void*), void* extra_arg)
| | ^~~~~~~~~~~~~~~
| | |
| | (5) entry to 'function_gadget'
| 409 | {
| 410 | output_gadget_t result = discarding_gadget();
| | ~~~~~~~~~~~~~~~~~~~
| | |
| | (6) calling 'discarding_gadget' from 'function_gadget'
|
+--> 'discarding_gadget': event 7
|
| 385 | static inline output_gadget_t discarding_gadget(void)
| | ^~~~~~~~~~~~~~~~~
| | |
| | (7) entry to 'discarding_gadget'
|
'discarding_gadget': event 8
|
| 390 | gadget.buffer = NULL;
| | ^
| | |
| | (8) 'gadget.buffer' is NULL
|
<------+
|
'function_gadget': event 9
|
| 410 | output_gadget_t result = discarding_gadget();
| | ^~~~~~~~~~~~~~~~~~~
| | |
| | (9) returning to 'function_gadget' from 'discarding_gadget'
|
<------+
|
'vfctprintf': events 10-11
|
| 1421 | output_gadget_t gadget = function_gadget(out, extra_arg);
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (10) returning to 'vfctprintf' from 'function_gadget'
| 1422 | return vsnprintf_impl(&gadget, format, arg);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (11) calling 'vsnprintf_impl' from 'vfctprintf'
|
+--> 'vsnprintf_impl': events 12-13
|
| 1387 | static int vsnprintf_impl(output_gadget_t* output, const char* format, va_list args)
| | ^~~~~~~~~~~~~~
| | |
| | (12) entry to 'vsnprintf_impl'
|......
| 1391 | format_string_loop(output, format, args);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (13) calling 'format_string_loop' from 'vsnprintf_impl'
|
+--> 'format_string_loop': events 14-19
|
| 1074 | static inline void format_string_loop(output_gadget_t* output, const char* format, va_list args)
| | ^~~~~~~~~~~~~~~~~~
| | |
| | (14) entry to 'format_string_loop'
|......
| 1083 | while (*format)
| | ~
| | |
| | (15) following 'true' branch...
| 1084 | {
| 1085 | if (*format != '%') {
| | ~~ ~
| | | |
| | | (17) following 'true' branch...
| | (16) ...to here
| 1086 | // A regular content character
| 1087 | putchar_via_gadget(output, *format);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (18) ...to here
| | (19) calling 'putchar_via_gadget' from 'format_string_loop'
|
+--> 'putchar_via_gadget': events 20-30
|
| 345 | static inline void putchar_via_gadget(output_gadget_t* gadget, char c)
| | ^~~~~~~~~~~~~~~~~~
| | |
| | (20) entry to 'putchar_via_gadget'
|......
| 350 | if (write_pos >= gadget->max_chars) {
| | ~
| | |
| | (21) following 'false' branch...
|......
| 353 | if (gadget->function != NULL) {
| | ~~ ~
| | | |
| | | (23) following 'false' branch...
| | (22) ...to here
|......
| 360 | gadget->buffer[write_pos] = c;
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | | | | |
| | | | | (30) dereference of NULL '*gadget.buffer + *gadget.pos'
| | | | (26) 'gadget.buffer' is NULL
| | | | (28) 'gadget.buffer' is NULL
| | | | (29) 'gadget.buffer' is NULL
| | | (25) 'gadget.buffer' is NULL
| | | (27) 'gadget.buffer' is NULL
| | (24) ...to here
|
It looks like you are calling fctprintf() with a NULL instead of your output function. Is it your intention to discard the output?
I'm not actually calling anything yet.
This is generated by the GCC 12.2Rel1 when it is doing its analysis of the printf.c file.
I think that the underlying issue is that the out parameter to vfctprintf is never checked to ensure that it is not NULL, so the code path indicated by the compiler could happen.
Hmm. So, the assignment into a NULL buffer can happen if the function gadget was created with a NULL function pointer. This only happens if the user invoked fctprintf() with a NULL function pointer. This library does not, generally, protect against dumb inputs. So, you'll also get a null pointer dereference if you write printf(NULL), because I execute while(*format), in the format_string_loop().
So, basically - it's not a bug, just a potential foot-gun, which is a design decision older than my involvement in this project.
So, basically - it's not a bug, just a potential foot-gun, which is a design decision older than my involvement in this project.
It also prevents compiling on GCC 12.2Rel1 with -Werror.
The warning goes away if I check the out parameter for NULL in vfctprintf but I'm not sure what the return value should be in that case.
@Rob-McKay : Is there a preprocessor macro which I can use to identify this compiler? Because GCC 12.2 on x86_64 doesn't complain about the issue. If I could pin down your compiler, I could add a warning suppression at that line.
The issue is a excessive warning with your compiler. I'll be happy to take a PR to address that issue - but certainly not to hurt performance in the innermost loop.
@eyalroz: I have found out what causes GCC to generate the warning. If you add the -fanalyzer compiler option you will also see the warning on x86_64.