Paint_Clear() writes 2*width bytes past end of framebuffer at Scale 65

[timmg]

Paint_Clear() writes 2 * width bytes past the end of the caller's buffer whenever Paint_SetScale(65) is in use. It happens on every call, for every width and height, and it corrupts whatever memory follows the framebuffer.

On our board that memory happened to be TinyUSB's endpoint state, so the device stopped enumerating over USB entirely — but the bug itself is generic and has nothing to do with USB.

The code

c/lib/GUI/GUI_Paint.c, Paint_Clear():

}else if(Paint.Scale == 65) {
    for (UWORD Y = 0; Y < Paint.HeightByte; Y++) {
        for (UWORD X = 0; X < Paint.WidthByte; X++ ) {//8 pixel =  1 byte
            UDOUBLE Addr = X*2 + Y*Paint.WidthByte;
            Paint.Image[Addr] = 0xff & (Color>>8);
            Paint.Image[Addr+1] = 0xff & Color;
        }
    }
}

Paint_SetScale() sets, for scale 65:

Paint.WidthByte = Paint.WidthMemory*2;

So WidthByte is a byte count (640 for a 320px-wide panel), but the inner loop uses it as a pixel count. The body then multiplies by 2 again to get a byte offset. Each row therefore writes 1280 bytes on a 640-byte stride, and the last row runs off the end:

last index written = 2WH + 2W - 1
last valid index   = 2WH - 1
overrun            = 2W bytes

The rows also overlap each other two-to-one, so every pixel is written twice — harmless for a uniform clear, but it is the same off-by-two.

Reproduction

Self-contained, no Pico SDK or Waveshare headers needed. The three functions are copied from the repository with only the relevant arithmetic kept.

/* cc -O2 -o repro repro.c && ./repro
 * Build with -DSLACK=0 -fsanitize=address to let ASan catch the write. */
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>

typedef uint8_t  UBYTE;
typedef uint16_t UWORD;
typedef uint32_t UDOUBLE;

/* ---------------- copied from GUI_Paint.c ---------------- */
static struct {
    UBYTE *Image;
    UWORD  WidthMemory, HeightMemory, WidthByte, HeightByte, Scale;
} Paint;

static void Paint_NewImage(UBYTE *image, UWORD Width, UWORD Height)
{
    Paint.Image        = image;
    Paint.WidthMemory  = Width;
    Paint.HeightMemory = Height;
    Paint.Scale        = 2;
    Paint.WidthByte    = (Width % 8 == 0) ? (Width / 8) : (Width / 8 + 1);
    Paint.HeightByte   = Height;
}

static void Paint_SetScale(UBYTE scale)
{
    if (scale == 65) {
        Paint.Scale     = scale;
        Paint.WidthByte = Paint.WidthMemory * 2;   /* NB: a BYTE count */
    }
}

static void Paint_Clear(UWORD Color)
{
    for (UWORD Y = 0; Y < Paint.HeightByte; Y++) {
        for (UWORD X = 0; X < Paint.WidthByte; X++) {  /* byte count used as pixel count */
            UDOUBLE Addr = X * 2 + Y * Paint.WidthByte;
            Paint.Image[Addr]     = 0xff & (Color >> 8);
            Paint.Image[Addr + 1] = 0xff & Color;
        }
    }
}
/* ---------------- end copied code ---------------- */

#ifndef SLACK
#define SLACK 8192   /* over-allocated so this repro cannot corrupt its own heap */
#endif

static int check(UWORD w, UWORD h)
{
    size_t fb = (size_t)w * h * 2;
    UBYTE *buf = calloc(1, fb + SLACK);
    if (!buf) return -1;

    Paint_NewImage(buf, w, h);
    Paint_SetScale(65);
    Paint_Clear(0x0862);

    size_t past = 0;
    for (size_t i = fb; i < fb + SLACK; i++)
        if (buf[i]) past = i - fb + 1;

    printf("  %4u x %-4u  buffer %7zu bytes   wrote %4zu bytes past the end   (2*width = %u)\n",
           w, h, fb, past, (unsigned)(2 * w));
    free(buf);
    return past != 0;
}

int main(void)
{
    printf("Paint_Clear(), Scale == 65 - bytes written past end of caller's buffer\n\n");
    int bad = 0;
    bad |= check(320, 172);  /* RP2350-LCD-1.47  */
    bad |= check(240, 240);  /* 1.28in round LCD */
    bad |= check(320, 240);  /* 2.0in LCD        */
    bad |= check(128, 128);  /* 1.44in LCD       */
    bad |= check(64, 32);    /* arbitrary        */
    return bad;
}

Output:

Paint_Clear(), Scale == 65 - bytes written past end of caller's buffer

   320 x 172   buffer  110080 bytes   wrote  640 bytes past the end   (2*width = 640)
   240 x 240   buffer  115200 bytes   wrote  480 bytes past the end   (2*width = 480)
   320 x 240   buffer  153600 bytes   wrote  640 bytes past the end   (2*width = 640)
   128 x 128   buffer   32768 bytes   wrote  256 bytes past the end   (2*width = 256)
    64 x 32    buffer    4096 bytes   wrote  128 bytes past the end   (2*width = 128)

Under AddressSanitizer with an exactly-sized buffer (-DSLACK=0 -fsanitize=address):

==10041==ERROR: AddressSanitizer: heap-buffer-overflow
WRITE of size 1 at 0x63300001b600 thread T0
0x63300001b600 is located 0 bytes after 110080-byte region [0x633000000800,0x63300001b600)

Fix

One line — bound the loop by Paint.WidthMemory, which counts pixels, matching what the body expects:

-            for (UWORD X = 0; X < Paint.WidthByte; X++ ) {//8 pixel =  1 byte
+            for (UWORD X = 0; X < Paint.WidthMemory; X++ ) {//1 pixel = 2 bytes

With that change the repro reports 0 bytes past the end at every size, ASan is clean, and the visible buffer is still fully covered (verified: 0 of 110080 bytes left unwritten, no gaps).

PR: #15

Why this is easy to miss

Waveshare's own examples malloc() the framebuffer, so the overrun lands in heap slack or allocator metadata and usually produces nothing visible. It only becomes obvious when the framebuffer is a static array with something important next to it. In our case the linker placed TinyUSB's hw_endpoints[] immediately after it:

framebuffer   0x20001dd0  +0x1ae00  -> ends 0x2001cbd0
hw_endpoints  0x2001cc00   0x400             <- 592 bytes overwritten every redraw

That destroyed EP0 milliseconds after tusb_init(), so the board asserted its D+ pull-up but could never answer a SETUP packet. The host gave up before enabling the port, and the device never appeared on the bus at all — no enumeration attempt, nothing in the kernel log. Took a while to trace back to a display function.

Related: Paint_SetPixel() bounds checks are off by one

Separate and much less severe, but in the same file:

if(Xpoint > Paint.Width || Ypoint > Paint.Height){        /* should be >= */
if(X > Paint.WidthMemory || Y > Paint.HeightMemory){      /* should be >= */

Paint.Width and Paint.Height are counts, so the last valid index is one less. As written, Xpoint == Paint.Width and Ypoint == Paint.Height pass the check and write 2 bytes out of bounds at scale 65.

I left this out of the PR to keep the diff to the one serious bug, but it is a two-character change if you want it in. Note it also appears in waveshareteam/e-Paper, so it may be worth fixing across the shared GUI_Paint.c lineage.