Skip to content

Print::printf returns an incorrect byte count #72

@l5yth

Description

@l5yth

Print::printf returns an incorrect byte count (and a huge garbage value on vsnprintf error)

Summary

Print::printf stores vsnprintf's int result in a size_t and returns it. Two correctness problems follow:

  1. Wrong count on truncation. vsnprintf returns the number of bytes it would have written, not what it actually wrote. When the formatted string exceeds the 256-byte stack buffer it is silently truncated, but the return value still reports the full would-be length, so callers that sum return values (to track bytes emitted) overcount.
  2. Garbage value on error. vsnprintf returns a negative int on an encoding error. Assigned to size_t n, that becomes a huge value (~SIZE_MAX), which is then returned to the caller.

Not a memory-safety bug (clarification)

The emitted output is not over-read: write(buf) resolves to Print::write(const char*), which writes strlen(buf) bytes, and vsnprintf always NUL-terminates within the buffer. So this is a return-value / truncation correctness issue, not an out-of-bounds read. (Flagging this explicitly because a sibling implementation that passed the length explicitly, write(buf, n), did over-read; portduino's write(buf) form avoids that.)

Affected version

meshtastic/framework-portduino @ 32b7b2c (HEAD, 2026-05-01).

Location

cores/portduino/PortduinoPrint.cpp:8-17:

size_t Print::printf(const char *format, ...) {
    char buf[MAX_STR_LEN]; // 256
    va_list args;
    va_start(args, format);
    size_t n = vsnprintf(buf, sizeof(buf), format, args);  // int -> size_t; negative becomes huge
    write(buf);                                            // writes strlen(buf), ignores n
    va_end(args);
    return n;                                              // may overstate, or be ~SIZE_MAX
}

Expected

Return the number of bytes actually written; never return a value larger than the buffer, and never a wrapped-negative.

Suggested fix

Keep the vsnprintf result as a signed int, handle the error and truncation cases, and write/return the clamped count:

- size_t n = vsnprintf(buf, sizeof(buf), format, args);
- write(buf);
- va_end(args);
- return n;
+ int n = vsnprintf(buf, sizeof(buf), format, args);
+ va_end(args);
+ if (n <= 0) return 0;
+ // vsnprintf returns the would-be length; clamp to what actually fit.
+ size_t len = (n < (int)sizeof(buf)) ? (size_t)n : sizeof(buf) - 1;
+ return write((const uint8_t *)buf, len);

(If silent truncation of long output is itself a concern, that's worth a separate decision, heap-allocate on overflow, or document the 255-byte cap.)

Severity

Low. Output is correct up to 255 bytes; the impact is a misleading/garbage return value, which matters only for callers that rely on it.

Notes

Found while working on ArduLinux, a downstream continuation; the clamp above (verified under AddressSanitizer) is what landed there.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions