Paint_SetPixel() bounds checks are off by one, allowing out-of-bounds writes

[timmg]

Both bounds checks in Paint_SetPixel() use > where they need >=, so a coordinate equal to the width or height passes validation and writes one row/column outside the caller's buffer. All 42 copies of GUI_Paint.c in this repository are affected.

The code

void Paint_SetPixel(UWORD Xpoint, UWORD Ypoint, UWORD Color)
{
    if(Xpoint > Paint.Width || Ypoint > Paint.Height){      /* should be >= */
        Debug("Exceeding display boundaries\r\n");
        return;
    }
    ...
    if(X > Paint.WidthMemory || Y > Paint.HeightMemory){    /* should be >= */
        Debug("Exceeding display boundaries\r\n");
        return;
    }

Paint.Width / Paint.Height and Paint.WidthMemory / Paint.HeightMemory are counts, so the last valid index is one less. As written, Xpoint == Paint.Width and Ypoint == Paint.Height are accepted. At Scale == 2 the address is then X / 8 + Y * Paint.WidthByte, which for X == Width lands exactly one byte past the row, and for Y == Height one full row past the buffer.

Reachable from the public API

This is not only a defensive-check problem — a normal call reaches it:

1. Paint_DrawRectangle() validates its own arguments with > as well, so Xend == Paint.Width is accepted.
2. It calls Paint_DrawLine(Xend, Yend, Xend, Ystart, ...).
3. Paint_DrawPoint()'s default dot style writes at Paint_SetPixel(Xpoint + XDir_Num - 1, ...). At DOT_PIXEL_2X2 or larger, XDir_Num reaches 1, so Paint_SetPixel() is called with Xpoint itself — i.e. Paint.Width.
4. The > guard lets it through, and the write goes past the end.

At DOT_PIXEL_1X1 the - 1 happens to pull the coordinate back in bounds, which is why this mostly goes unnoticed.

Reproduction

Compiles the unmodified RaspberryPi_JetsonNano/c/lib/GUI/GUI_Paint.c. The only extra pieces are stub DEV_Config.h / Debug.h providing the typedefs, so it builds off-target:

/* DEV_Config.h stub */
#define UBYTE   uint8_t
#define UWORD   uint16_t
#define UDOUBLE uint32_t

/* Debug.h stub */
#define Debug(...) do{}while(0)

#include "GUI_Paint.h"
#define W 128
#define H 296                                  /* 2.9in e-Paper */

int main(void) {
    UBYTE *img = malloc((W / 8) * H);          /* 4736 bytes, exactly as documented */
    Paint_NewImage(img, W, H, ROTATE_0, WHITE);
    Paint_SelectImage(img);
    Paint_SetScale(2);
    Paint_Clear(WHITE);
    Paint_DrawRectangle(0, 0, W, H, BLACK, DOT_PIXEL_2X2, DRAW_FILL_EMPTY);
    return 0;
}

cc -O1 -g -fsanitize=address -I stub -I lib/GUI -I lib/Fonts \
   -o border border.c lib/GUI/GUI_Paint.c && ./border

==13923==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x621000001380
READ of size 1 at 0x621000001380 thread T0
    #0 Paint_SetPixel      GUI_Paint.c:241
    #1 Paint_DrawLine      GUI_Paint.c:388
    #2 Paint_DrawRectangle GUI_Paint.c:432
    #3 main                border.c:22

0x621000001380 is located 0 bytes after 4736-byte region [0x621000000100,0x621000001380)

(ASan reports the read of the read-modify-write at GUI_Paint.c:241; the write to the same address follows.)

Fix

-    if(Xpoint > Paint.Width || Ypoint > Paint.Height){
+    if(Xpoint >= Paint.Width || Ypoint >= Paint.Height){
...
-    if(X > Paint.WidthMemory || Y > Paint.HeightMemory){
+    if(X >= Paint.WidthMemory || Y >= Paint.HeightMemory){

Paint_SetPixel() is the single point every pixel write passes through, so correcting it there protects all the drawing primitives without touching them individually.

With the change the reproduction above is ASan-clean, and rendering is unaffected — the guard now rejects only writes that were already out of bounds. Verified by rendering a scene with in-range coordinates only (filled and unfilled rectangles, DOT_PIXEL_3X3 and dotted lines, filled and unfilled circles, DOT_FILL_AROUND points along the last row) against both versions and comparing the buffers: byte-identical, 4736 bytes.

One behavioural note: code that called Paint_DrawRectangle(0, 0, W, H, ...) will now have that last row/column clipped instead of written out of bounds. W-1, H-1 is the correct call and is unaffected.

PR: #426 — applies this to all 42 copies, byte-identical in each.

Related

waveshare/Pico_code has the same off-by-one, plus a more serious overrun in Paint_Clear() at Scale == 65 that writes 2 * width bytes past the end of the buffer on every call. Reported there as waveshare/Pico_code#16 with a fix in waveshare/Pico_code#15. The Scale == 65 branch does not exist in this repository, so only the off-by-one applies here.