Skip to content

Fix buffer overrun in Paint_Clear() at Scale 65 - #15

Open
timmg wants to merge 1 commit into
waveshare:mainfrom
timmg:fix-paint-clear-overrun
Open

Fix buffer overrun in Paint_Clear() at Scale 65#15
timmg wants to merge 1 commit into
waveshare:mainfrom
timmg:fix-paint-clear-overrun

Conversation

@timmg

@timmg timmg commented Aug 5, 2026

Copy link
Copy Markdown

Paint_Clear() writes 2 * width bytes past the end of the caller's buffer whenever Paint_SetScale(65) is in use — on every call, at every width and height.

Paint_SetScale() sets Paint.WidthByte = Paint.WidthMemory*2 for scale 65, so it holds a byte count (640 for a 320px panel). The inner loop of Paint_Clear() used it as a pixel count, and the body then multiplies by 2 again to form the 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

This changes the bound to Paint.WidthMemory, which counts pixels — matching what the loop body expects. One line.

Verification

A standalone reproduction (no SDK needed, code in the linked issue) reports, before the change:

   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)

and with an exactly-sized buffer under AddressSanitizer:

AddressSanitizer: heap-buffer-overflow
WRITE of size 1 ... located 0 bytes after 110080-byte region

After the change: 0 bytes past the end at every size, ASan clean, and the visible buffer is still covered exactly — verified 0 of 110080 bytes left unwritten, no gaps and no change to the rendered result.

Impact

Waveshare's own examples malloc() the framebuffer, so the overrun usually lands in heap slack and produces nothing visible. It becomes serious when the framebuffer is a static array. On an RP2350 board the linker placed TinyUSB's hw_endpoints[] directly after ours, so every redraw destroyed EP0: the board asserted its D+ pull-up but could never answer a SETUP packet, and never appeared on the USB bus at all.

🤖 Generated with Claude Code

Fixes #16 — that issue has the full write-up and a standalone reproduction.

The inner loop was bounded by Paint.WidthByte, which Paint_SetScale(65)
sets to WidthMemory*2 - a byte count, not a pixel count. Combined with
the Addr = X*2 below it, each row wrote 1280 bytes on a 640-byte stride
(at width 320), so the final row ran 2*width bytes past the end of the
caller's buffer.

The overrun happens for every width and height, on every call. Bounding
the loop by Paint.WidthMemory counts pixels, which is what the body
expects, and still covers the buffer exactly with no gaps.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant