Validate DGUS read variable lengths

[quietcode-org]

Problem

The DGUS READVAR handlers could access the VP address, word count, or payload before confirming that those bytes were present. Truncated or over-declared frames could therefore read beyond the received datagram in the Classic, E3S1Pro, and Reloaded implementations.

Fix

Require READVAR frames to contain the VP address and word count, and require the declared payload length to match the received payload before accessing it.

The E3S1Pro and Reloaded string paths now keep the payload length wide enough for the declared word count. Valid three-byte 0x82 / OK acknowledgements are unchanged.

Validation

Malformed and over-declared READVAR frames reproduced out-of-bounds reads before the fix and are rejected after it.

• mega2560ext, test 2 (DGUS MKS): passed
• mega2560ext, test 3 (DGUS Reloaded): passed
• STM32F401RC_creality, test 99 (DGUS E3S1Pro): passed
• Flash: +48 bytes Classic, +40 bytes Reloaded, +24 bytes E3S1Pro
• RAM: unchanged

No device was accessed.

[ellensp]

@quietcode-org, please note
This is a community-run codebase; we don't have hundreds of employees to check out all these PRs.
Officially, there is 1 person doing all the work and random volunteers like me that come and go.

Note: Marlin code is for microcontrollers. It does not need such validation for every single thing. That could increase the codebase to exceed the limited resources of the microcontrollers. As long as the code handles errors ok, that is all that is required.

[cyberman]

quietcode-org ist my coding account, cyberman is for research and management.

Thank you for taking the time to point this out, especially given how little maintainer capacity Marlin currently has. I do understand that every additional review request consumes time that is already in very short supply.

I also agree with your broader point: Marlin runs on constrained microcontrollers, including AVR targets, so indiscriminate defensive validation would be the wrong direction. I am not trying to add checks for every theoretically possible condition.

The reason I considered this particular validation important is that it sits directly at the serial protocol boundary. The READVAR handlers accessed the address, word-count, and payload fields before establishing that those bytes had actually been received. With truncated or over-declared frames, I was able to reproduce out-of-bounds reads. The added checks therefore only verify the minimum frame structure and that the declared payload length matches the received payload before it is accessed. Valid acknowledgements and valid READVAR frames remain on the existing path.

Your concern about code size prompted me to measure the actual binaries rather than rely on intuition. The results were:

• Classic DGUS on mega2560ext: +48 bytes Flash, 0 bytes RAM
• DGUS Reloaded on mega2560ext: +40 bytes Flash, 0 bytes RAM
• DGUS E3S1Pro on STM32F401RC_creality: +24 bytes Flash, 0 bytes RAM

So the maximum added cost in any one configuration is 48 bytes, not the sum of all three variants.

Part of my background is AmigaOS, where treating memory and CPU time as scarce resources—and making every added byte justify itself—is deeply ingrained. That is also why I take the AVR concern seriously. At the same time, that background has taught me to be cautious at external data boundaries, because a very small check there can prevent failures that are much harder to diagnose later.

I hope the measurements help put the trade-off into perspective. I am certainly open to a smaller implementation if the same boundary can be protected with fewer bytes, but I believe leaving the demonstrated out-of-bounds reads unhandled would be the less robust choice.