From 34707966082f05b822b4e290ea97c09eb8a56536 Mon Sep 17 00:00:00 2001 From: TD-er Date: Thu, 6 Feb 2025 13:42:50 +0100 Subject: [PATCH 1/2] Fix potential out of range in String::concat There is no prerequisite the given array has to be a 0-terminated char array. So we should only copy the length that has been given. The `setLen()` function will make sure the internal string is 0-terminated. So no need to dangerously assume there will be 1 more byte to copy --- cores/esp32/WString.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cores/esp32/WString.cpp b/cores/esp32/WString.cpp index 71183213ac2..6d159ce21f0 100644 --- a/cores/esp32/WString.cpp +++ b/cores/esp32/WString.cpp @@ -343,10 +343,10 @@ bool String::concat(const char *cstr, unsigned int length) { } if (cstr >= wbuffer() && cstr < wbuffer() + len()) { // compatible with SSO in ram #6155 (case "x += x.c_str()") - memmove(wbuffer() + len(), cstr, length + 1); + memmove(wbuffer() + len(), cstr, length); } else { // compatible with source in flash #6367 - memcpy_P(wbuffer() + len(), cstr, length + 1); + memcpy_P(wbuffer() + len(), cstr, length); } setLen(newlen); return true; From 998ce4c496f8d06f444917702ff247a7480c8a50 Mon Sep 17 00:00:00 2001 From: TD-er Date: Thu, 6 Feb 2025 14:27:44 +0100 Subject: [PATCH 2/2] Allow String::concat(const String &s) with s.buffer() == nullptr When constructing a String object, the internal buffer is a nullptr. However concatenating this to another String would return `false` while this is perfectly fine to do. --- cores/esp32/WString.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/cores/esp32/WString.cpp b/cores/esp32/WString.cpp index 6d159ce21f0..f7381b3ac10 100644 --- a/cores/esp32/WString.cpp +++ b/cores/esp32/WString.cpp @@ -311,23 +311,21 @@ bool String::concat(const String &s) { // Special case if we're concatting ourself (s += s;) since we may end up // realloc'ing the buffer and moving s.buffer in the method called if (&s == this) { - unsigned int newlen = 2 * len(); - if (!s.buffer()) { - return false; - } if (s.len() == 0) { return true; } + if (!s.buffer()) { + return false; + } + unsigned int newlen = 2 * len(); if (!reserve(newlen)) { return false; } memmove(wbuffer() + len(), buffer(), len()); setLen(newlen); - wbuffer()[len()] = 0; return true; - } else { - return concat(s.buffer(), s.len()); } + return concat(s.buffer(), s.len()); } bool String::concat(const char *cstr, unsigned int length) {