From e38e02daaf64c262484fd523a7213c3b90095a61 Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Wed, 10 Apr 2019 04:38:09 -0700 Subject: [PATCH 1/2] Fix String.replace overlapping strcpy Fixes #5949 Adds a test from the issue above and fixes the problem valgrind found. --- cores/esp8266/WString.cpp | 2 +- tests/host/core/test_string.cpp | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp index ad826976f1..ac2d1411aa 100644 --- a/cores/esp8266/WString.cpp +++ b/cores/esp8266/WString.cpp @@ -744,7 +744,7 @@ void String::replace(const String& find, const String& replace) { readFrom = foundAt + find.len(); setLen(len() + diff); } - strcpy(writeTo, readFrom); + memmove(writeTo, readFrom, strlen(readFrom)+1); } else { unsigned int size = len(); // compute size needed for result while((foundAt = strstr(readFrom, find.buffer())) != NULL) { diff --git a/tests/host/core/test_string.cpp b/tests/host/core/test_string.cpp index 9ffd6d4162..df08d9a00e 100644 --- a/tests/host/core/test_string.cpp +++ b/tests/host/core/test_string.cpp @@ -428,3 +428,15 @@ TEST_CASE("String SSO handles junk in memory", "[core][String]") REQUIRE(*s == "CO2_defect"); s->~String(); } + + +TEST_CASE("Issue #5949 - Overlapping src/dest in replace", "[core][String]") +{ + String blah = "blah"; + blah.replace("xx", "y"); + REQUIRE(blah == "blah"); + blah.replace("x", "yy"); + REQUIRE(blah == "blah"); + blah.replace(blah, blah); + REQUIRE(blah == "blah"); +} From 00e2ba78de47b5d4e1ab3add5e8444209138449a Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Wed, 10 Apr 2019 06:36:01 -0700 Subject: [PATCH 2/2] Additional pathological memcpy->memmove fixes --- cores/esp8266/WString.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp index ac2d1411aa..d949e2904e 100644 --- a/cores/esp8266/WString.cpp +++ b/cores/esp8266/WString.cpp @@ -237,7 +237,7 @@ void String::move(String &rhs) { } if (rhs.sso()) { setSSO(true); - memcpy(sso_buf, rhs.sso_buf, sizeof(sso_buf)); + memmove(sso_buf, rhs.sso_buf, sizeof(sso_buf)); } else { setSSO(false); setBuffer(rhs.wbuffer()); @@ -730,16 +730,16 @@ void String::replace(const String& find, const String& replace) { char *foundAt; if(diff == 0) { while((foundAt = strstr(readFrom, find.buffer())) != NULL) { - memcpy(foundAt, replace.buffer(), replace.len()); + memmove(foundAt, replace.buffer(), replace.len()); readFrom = foundAt + replace.len(); } } else if(diff < 0) { char *writeTo = wbuffer(); while((foundAt = strstr(readFrom, find.buffer())) != NULL) { unsigned int n = foundAt - readFrom; - memcpy(writeTo, readFrom, n); + memmove(writeTo, readFrom, n); writeTo += n; - memcpy(writeTo, replace.buffer(), replace.len()); + memmove(writeTo, replace.buffer(), replace.len()); writeTo += replace.len(); readFrom = foundAt + find.len(); setLen(len() + diff); @@ -760,7 +760,7 @@ void String::replace(const String& find, const String& replace) { readFrom = wbuffer() + index + find.len(); memmove(readFrom + diff, readFrom, len() - (readFrom - buffer())); int newLen = len() + diff; - memcpy(wbuffer() + index, replace.buffer(), replace.len()); + memmove(wbuffer() + index, replace.buffer(), replace.len()); setLen(newLen); wbuffer()[newLen] = 0; index--;