Fix out-of-bounds read in vsnprintf_async_signal_safe on a trailing '%'

[magic-peach]

Problem

vsnprintf_async_signal_safe() reads one or more bytes past the end of the
format string when the format ends with a lone conversion introducer — "%",
"%l" or "%ll".

After skipping % (and any l/ll length modifier), the code lands on the
terminating NUL. The switch on the conversion specifier has no case for
'\0' and no default, so it falls through, and the enclosing
for (; *format; ++format) loop then advances format past the NUL,
reading out of bounds on the next *format test.

This function backs serverLogFromHandler() (the async-signal-safe logger used
from signal handlers), so it should stay robust against malformed format
strings rather than read out of bounds.

Fix

Stop processing when the conversion introducer is immediately followed by the
end of the format string:

format = check_longlong_async_signal_safe(format, &have_longlong);

/* A malformed trailing conversion (e.g. "%", "%l" or "%ll" at the end
 * of the format) leaves us on the terminating NUL. Stop here, otherwise
 * the loop's ++format would step past the end of the string. */
if (*format == '\0') break;

Well-formed conversions are unaffected: their specifier character is never the
NUL, so the guard never triggers for them.

Tests

Added TestSnprintfAsyncSignalSafe in src/unit/test_util.cpp (there was no
existing coverage for these functions). It checks ordinary formatting (%d,
%s) plus the trailing-%, %l and %ll cases.

Verified the out-of-bounds read against the current code by extracting the
function into a standalone harness built with -fsanitize=address:

==...==ERROR: AddressSanitizer: stack-buffer-overflow ... READ of size 1
    'f1' ... <== Memory access ... overflows this variable
SUMMARY: AddressSanitizer: stack-buffer-overflow ... in vsnprintf_async_signal_safe

With the fix, all cases pass with no sanitizer error.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2ae3e2f3-d456-4df6-a51a-75319eb8f243

📥 Commits

Reviewing files that changed from the base of the PR and between 6c315de and d4cec91.

📒 Files selected for processing (2)

• src/unit/test_util.cpp
• src/util.c

---

📝 Walkthrough

Walkthrough

Fixes vsnprintf_async_signal_safe() in src/util.c to stop the format-scanning loop when the format string terminates after a malformed length modifier, preventing reads past the string end. Adds a unit test in test_util.cpp validating this behavior for various malformed format inputs.

Changes

Malformed Format String Fix

Layer / File(s)	Summary
Stop scanning on malformed trailing conversions src/util.c	Adds an early break when the format pointer reaches the terminating NUL after handling %, %l, or %ll, preventing the subsequent switch statement from reading past the format string.
Test for safe malformed format handling src/unit/test_util.cpp	Adds <cstdarg> include, a variadic callSnprintfAsyncSignalSafe wrapper to bypass printf-format warnings, and a new test verifying correct output for valid formats and safe zero-return/empty-output behavior for malformed formats like trailing %, %l, and %ll.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main fix: preventing an out-of-bounds read in vsnprintf_async_signal_safe on malformed trailing format strings.
Description check	✅ Passed	The description clearly matches the changeset, explaining the bug, fix, and added tests for the affected formatting cases.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[valkey-review-bot]

One test-coverage issue below: the malformed-format cases can still pass against the old implementation in a non-ASAN build.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.77%. Comparing base (6c315de) to head (d4cec91).
⚠️ Report is 16 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #4129      +/-   ##
============================================
- Coverage     76.80%   76.77%   -0.03%     
============================================
  Files           162      162              
  Lines         81169    81190      +21     
============================================
- Hits          62339    62337       -2     
- Misses        18830    18853      +23     

Files with missing lines	Coverage Δ
src/unit/test_util.cpp	89.38% <100.00%> (+1.03%)	⬆️
src/util.c	72.05% <100.00%> (+0.29%)	⬆️

... and 16 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.