From 54240d2cc51e8b134fc6dc583f5a2b21f19755d8 Mon Sep 17 00:00:00 2001
From: "Earle F. Philhower, III" <earlephilhower@yahoo.com>
Date: Wed, 20 Mar 2019 06:18:04 -0700
Subject: [PATCH] Fix String::replace() garbage at end of string (#5897)

* Fix String::replace()

Fixes #5883 and supercedes #5890

The replace() function was using len() while in the middle of buffer
operations.  In SSO mode len() is not stored separately and is a call to
strlen(), which may not be legal if you're in the middle of overwriting
the SSO buffer, as was the case in ::replace when the replacement string
was longer than the find string.  This caused potential garbage at the
end of the string when accessed.  Instead, just cache the length in a
local while doing the operation.

Add in test cases from #5890 as well as some new ones that fail on the
unmodified core.

* Fix stack smashing error on 64b

When pointers are 8 bytes long, the size of a String is larger than 16
chars.  Increase the allocated array we're using in the test to avoid a
"stack smashing" error.

* Manually call destructor in test

Just for clarity, manually call the destructor for the Strings() that
are "placement new'd" in the String tests.  It is a no-op for the
existing test, since thanks to SSO there are no memory allocations, but
will help in case someone adds tests later which include longer strings.
---
 cores/esp8266/WString.cpp       |  5 ++-
 tests/host/core/test_string.cpp | 67 +++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp
index 4d4fa3bf2..ad826976f 100644
--- a/cores/esp8266/WString.cpp
+++ b/cores/esp8266/WString.cpp
@@ -759,9 +759,10 @@ void String::replace(const String& find, const String& replace) {
         while(index >= 0 && (index = lastIndexOf(find, index)) >= 0) {
             readFrom = wbuffer() + index + find.len();
             memmove(readFrom + diff, readFrom, len() - (readFrom - buffer()));
-            setLen(len() + diff);
-            wbuffer()[len()] = 0;
+	    int newLen = len() + diff;
             memcpy(wbuffer() + index, replace.buffer(), replace.len());
+            setLen(newLen);
+            wbuffer()[newLen] = 0;
             index--;
         }
     }
diff --git a/tests/host/core/test_string.cpp b/tests/host/core/test_string.cpp
index c2ef68025..9ffd6d416 100644
--- a/tests/host/core/test_string.cpp
+++ b/tests/host/core/test_string.cpp
@@ -83,6 +83,12 @@ TEST_CASE("String constructors", "[core][String]")
     REQUIRE(ssh == "3.14159_abcd");
     String flash = (F("hello from flash"));
     REQUIRE(flash == "hello from flash");
+    const char textarray[6] = {'h', 'e', 'l', 'l', 'o', 0};
+    String hello(textarray);
+    REQUIRE(hello == "hello");
+    String hello2;
+    hello2 = textarray;
+    REQUIRE(hello2 == "hello");
 }
 
 TEST_CASE("String concantenation", "[core][String]")
@@ -360,4 +366,65 @@ TEST_CASE("String SSO works", "[core][String]")
     REQUIRE(s == "0123456789abcdefghijklmnopqrstu");
     REQUIRE(s.length() == 31);
   }
+  s = "0123456789abcde";
+  s = s.substring(s.indexOf('a'));
+  REQUIRE(s == "abcde");
+  REQUIRE(s.length() == 5);
+}
+
+#include <new>
+void repl(const String& key, const String& val, String& s, boolean useURLencode)
+{
+    s.replace(key, val);
+}
+
+
+TEST_CASE("String SSO handles junk in memory", "[core][String]")
+{
+  // We fill the SSO space with garbage then construct an object in it and check consistency
+  // This is NOT how you want to use Strings outside of this testing!
+  unsigned char space[64];
+  String *s = (String*)space;
+  memset(space, 0xff, 64);
+  new(s) String;
+  REQUIRE(*s == "");
+  s->~String();
+
+  // Tests from #5883
+  bool useURLencode = false;
+  const char euro[4] = {(char)0xe2, (char)0x82, (char)0xac, 0}; // Unicode euro symbol
+  const char yen[3]   = {(char)0xc2, (char)0xa5, 0}; // Unicode yen symbol
+
+  memset(space, 0xff, 64);
+  new(s) String("%ssid%");
+  repl(("%ssid%"), "MikroTik", *s, useURLencode);
+  REQUIRE(*s == "MikroTik");
+  s->~String();
+
+  memset(space, 0xff, 64);
+  new(s) String("{E}");
+  repl(("{E}"), euro, *s, useURLencode);
+  REQUIRE(*s == "€");
+  s->~String();
+  memset(space, 0xff, 64);
+  new(s) String("&euro;");
+  repl(("&euro;"), euro, *s, useURLencode);
+  REQUIRE(*s == "€");
+  s->~String();
+  memset(space, 0xff, 64);
+  new(s) String("{Y}");
+  repl(("{Y}"), yen, *s, useURLencode);
+  REQUIRE(*s == "¥");
+  s->~String();
+  memset(space, 0xff, 64);
+  new(s) String("&yen;");
+  repl(("&yen;"), yen, *s, useURLencode);
+  REQUIRE(*s == "¥");
+  s->~String();
+
+  memset(space, 0xff, 64);
+  new(s) String("%sysname%");
+  repl(("%sysname%"), "CO2_defect", *s, useURLencode);
+  REQUIRE(*s == "CO2_defect");
+  s->~String();
 }