From a41ddec1a7368af384e53e85024ae22f5e88f342 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Wed, 12 Aug 2026 18:17:33 -0500 Subject: [PATCH] fix(nrf54l15): don't write past String buffer when a grow fails (#11454) * fix(nrf54l15): don't write past String buffer when a grow fails reserve() correctly keeps the old buffer when realloc returns NULL, but returned void, and assign()/concat() proceeded to memcpy with n >= _cap anyway - a heap overflow of up to n+1-_cap bytes into adjacent allocations. On this Zephyr target allocation failure is a realistic condition, and the result was heap corruption instead of a clean no-op. reserve() now reports success and the callers leave the string unchanged when the grow fails. * fix(nrf54l15): guard String length arithmetic against wraparound Per review: reject size requests whose n+1 / _len+n arithmetic would wrap before they reach the capacity check, and make reserve(0) fail without calling realloc (realloc(p, 0) would free the buffer and return NULL, leaving _buf dangling). --- src/platform/nrf54l15/Arduino.h | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/platform/nrf54l15/Arduino.h b/src/platform/nrf54l15/Arduino.h index c67628afa..0d7449e89 100644 --- a/src/platform/nrf54l15/Arduino.h +++ b/src/platform/nrf54l15/Arduino.h @@ -599,8 +599,12 @@ class String void assign(const char *s, unsigned int n) { - if (n >= _cap) - reserve(n + 1); + // reserve() keeps the old (smaller) buffer on OOM, so a failed grow must abort the + // write: memcpy'ing n >= _cap bytes would overflow into adjacent heap. + if (n + 1 == 0) + return; // n + 1 would wrap + if (n >= _cap && !reserve(n + 1)) + return; if (_buf) { memcpy(_buf, s, n); _buf[n] = 0; @@ -612,21 +616,27 @@ class String if (!s || n == 0) return; unsigned newlen = _len + n; - if (newlen >= _cap) - reserve(newlen + 1); + if (newlen < _len || newlen + 1 == 0) + return; // length arithmetic wrapped + if (newlen >= _cap && !reserve(newlen + 1)) + return; // OOM: keep the existing content intact instead of writing past the buffer if (_buf) { memcpy(_buf + _len, s, n); _len = newlen; _buf[_len] = 0; } } - void reserve(unsigned int n) + bool reserve(unsigned int n) { + if (n == 0) + return false; char *b = (char *)realloc(_buf, n); if (b) { _buf = b; _cap = n; + return true; } + return false; } };