-
Notifications
You must be signed in to change notification settings - Fork 159
Buffer overflow on _gx_system_draw_context_stack[GX_MAX_CONTEXT_NESTING] #148
Description
Describe the bug
tl;dr:
Somewhere within the GUIX/common files there must be a weakness in checking the limits of _gx_system_draw_context_stack[GX_MAX_CONTEXT_NESTING].
Until now I was unable to pin point the error due to the amount of GUIX code that might be involved and my lack of knowledge about it.
But I'll describe the effects I saw and steps I did so far to track down this error below.
I tend to classify this as a security issue!
long version:
- My micro controller produced Hard Fault interrupts and reset itself. This happened after I just added some more additional text prompt widgets with predefined strings in GUIX Studio. (So no line of custom code changed which directly pointed my attention to the GUIX library itself.)
- Naturally my first instinct was checking task stack utilization but there was no shortage. (Side note: I do not use any heap or dynamic memory allocation. Neither in FreeRTOS nor in GUIX. Hence heap could not be the issue. But due to the kind of error I still suspect some kind of overflow somewhere in GUIX.)
- Using breakpoints and single stepping through different parts of the GUIX library my first discovery was, that shortly before the Hard Fault happens GUIX was beginning a new drawing operation that started exactly at that position in the canvas memory ended. It didn't begin within and drew to far, it started on the RAM address just next to the last canvas address! (I use a display with "on-chip display data RAM" and only a partial frame buffer on my micro controller with
#define GX_ENABLE_CANVAS_PARTIAL_FRAME_BUFFER. The GUIX display driver I use is the gx_display_driver_16bpp initialized by_gx_display_driver_565rgb_setup(my_display, GX_NULL, my_toggle_function);) - I found out, that the display driver itself did nothing wrong but did what it was told to do by the parameters given to it during the function call. This let me to the "context" pointer given to the driver by GUIX.
- At this point I increased
GX_MAX_CONTEXT_NESTINGback to its default value of 8 which solved the Hard Fault on my project. (Since it's a documented define to configure GUIX and I need to save memory where ever possible I initially had#define GX_MAX_CONTEXT_NESTING 4in mygx_user.hfile.) - I tried to find out if some function doesn't obey the array size defined by
GX_MAX_CONTEXT_NESTINGand just assumes the default size of 8 but I was unable to find anything that supports this theory. Instead I collected a whole bunch of keywords in the search window of my IDE:(GX_MAX_CONTEXT_NESTING|_gx_system_draw_context_stack|_gx_system_current_draw_contextnew_context|current_context|context). For me this is a rabbit whole where I realized a kind of chaos to some extent and I was unable to grasp in a reasonable amount of time how it should be implemented within GUIX. That's why so far I can not pin point the exact issue within GUIX but
this seams to be a major issue that is not related to any fixed value but is a general issue with missing void pointer handling in various functions when using_gx_system_current_draw_context.
Furthermore I didn't understand yet which function is allowed to increase/decrease the index of _gx_system_draw_context_stack[] and how (or if at all ?!?!) the limits of this array are checked.
Currently my educated guess is that any system is effected by this bug if the context nesting is not big enough.
Please also mention any information which could help others to understand
the problem you're facing:
- GUIX tag: v6.5.0.202601_rel
- gx_user.h settings:
#define GX_DISABLE_THREADX_BINDING // -> FreeRTOS #define GX_DISABLE_UTF8_SUPPORT // Default: Not Defined. #define GX_DISABLE_ARC_DRAWING_SUPPORT // Default: Not defined. #define GX_DISABLE_SOFTWARE_DECODER_SUPPORT // Default: Not defined. #define GX_DISABLE_BINARY_RESOURCE_SUPPORT // Default: Not defined. #define GX_DISABLE_BRUSH_ALPHA_SUPPORT // Default: Not defined. #define GX_ENABLE_CANVAS_PARTIAL_FRAME_BUFFER // Default: Not defined. #define GX_MAX_QUEUE_EVENTS 24 // Default: 48. #define GX_MAX_DIRTY_AREAS 16 // Default: 64. #define GX_MAX_CONTEXT_NESTING 4 // Default: 8. -> This value is related to the issue. #define GX_MAX_INPUT_CAPTURE_NESTING 1 // Default: 4. #define GX_MULTI_LINE_INDEX_CACHE_SIZE 16 // Default: 32. #define GX_MULTI_LINE_TEXT_BUTTON_MAX_LINES 2 // Default: 4. #define GX_POLYGON_MAX_EDGE_NUM 4 // Default: 10. #define GX_NUMERIC_SCROLL_WHEEL_STRING_BUFFER_SIZE 4 // Default: 16. #define GX_NUMERIC_PROMPT_BUFFER_SIZE 4 // Default: 16. #define GX_ANIMATION_POOL_SIZE 0 // Default: 6. #define GX_MAX_STRING_LENGTH 2048 // Default: 102400 #define GX_THREAD_STACK_SIZE 512 // Default: 4096 #define GX_DISABLE_DEPRECATED_STRING_API // Default: Not defined.
To Reproduce
Steps to reproduce the behavior:
I guess it can be reproduced with any project. For instance I took guix/tutorials/demo_guix_transitions, changed GX_MAX_CONTEXT_NESTING and got an error, too.
Expected behavior
Changing any configuration flag documented in appendix-h should not cause weird errors like this.
Configured values should be checked against absolute maximum/minimum values given by the library at compile time.
At runtime range checks should give understandable error messages or just refuse to do anything but should never write out of bound.
Impact
Since sufficient range checks are clearly not in place here I tend to classify this as a security issue!