Fix memory corruption on x64

Because pointers are 8 bytes (and 8-bytes aligned) on x64, the structure
used to store SSO needs to make sure the terminal \0 won't overwrite
some of that buffer.  Adjust the SSOSIZE dynamically depending on the
size of the struct ptr.

Refactor to add a setBuffer method, like the setConfig, setSSO, etc.

Fix some issues found when using SSO strings and functions which modify
len() dynamically.

Author: earlephilhower

cores/esp8266/WString.cpp

@@ -132,12 +132,12 @@ inline void String::init(void) {
     setSSO(false);
     setCapacity(0);
     setLen(0);
-    ptr.buf = NULL;
+    setBuffer(nullptr);
 }
 
 void String::invalidate(void) {
-    if(!sso() && ptr.buf)
-        free(ptr.buf);
+    if(!sso() && wbuffer())
+        free(wbuffer());
     init();
 }
 
@@ -155,15 +155,15 @@ unsigned char String::reserve(unsigned int size) {
 unsigned char String::changeBuffer(unsigned int maxStrLen) {
     // Can we use SSO here to avoid allocation?
     if (maxStrLen < sizeof(sso_buf)) {
-        if (sso() || !ptr.buf) {
+        if (sso() || !buffer()) {
             // Already using SSO, nothing to do
             setSSO(true);
             return 1;
         } else { // if bufptr && !sso()
             // Using bufptr, need to shrink into sso_buff
             char temp[sizeof(sso_buf)];
             memcpy(temp, buffer(), maxStrLen);
-            free(ptr.buf);
+            free(wbuffer());
             setSSO(true);
             memcpy(wbuffer(), temp, maxStrLen);
             return 1;
@@ -176,7 +176,7 @@ unsigned char String::changeBuffer(unsigned int maxStrLen) {
         return false;
     }
     uint16_t oldLen = len();
-    char *newbuffer = (char *) realloc(sso() ? nullptr : ptr.buf, newSize);
+    char *newbuffer = (char *) realloc(sso() ? nullptr : wbuffer(), newSize);
     if(newbuffer) {
         size_t oldSize = capacity() + 1; // include NULL.
         if (sso()) {
@@ -190,7 +190,7 @@ unsigned char String::changeBuffer(unsigned int maxStrLen) {
         setSSO(false);
         setCapacity(newSize - 1);
         setLen(oldLen); // Needed in case of SSO where len() never existed
-        ptr.buf = newbuffer;
+        setBuffer(newbuffer);
         return 1;
     }
     return 0;
@@ -230,8 +230,8 @@ void String::move(String &rhs) {
             return;
         } else {
             if (!sso()) {
-                free(ptr.buf);
-                ptr.buf = nullptr;
+                free(wbuffer());
+                setBuffer(nullptr);
             }
         }
     }
@@ -240,14 +240,14 @@ void String::move(String &rhs) {
         memcpy(sso_buf, rhs.sso_buf, sizeof(sso_buf));
     } else {
         setSSO(false);
-        ptr.buf = rhs.ptr.buf;
+        setBuffer(rhs.wbuffer());
     }
     setCapacity(rhs.capacity());
     setLen(rhs.len());
     rhs.setSSO(false);
     rhs.setCapacity(0);
     rhs.setLen(0);
-    rhs.ptr.buf = nullptr;
+    rhs.setBuffer(nullptr);
 }
 #endif
 
@@ -785,9 +785,10 @@ void String::remove(unsigned int index, unsigned int count) {
         count = len() - index;
     }
     char *writeTo = wbuffer() + index;
-    setLen(len() - count);
-    memmove(writeTo, wbuffer() + index + count, len() - index);
-    wbuffer()[len()] = 0;
+    unsigned int newlen = len() - count;
+    setLen(newlen);
+    memmove(writeTo, wbuffer() + index + count, newlen - index);
+    wbuffer()[newlen] = 0;
 }
 
 void String::toLowerCase(void) {
@@ -815,10 +816,11 @@ void String::trim(void) {
     char *end = wbuffer() + len() - 1;
     while(isspace(*end) && end >= begin)
         end--;
-    setLen(end + 1 - begin);
+    unsigned int newlen = end + 1 - begin;
+    setLen(newlen);
     if(begin > buffer())
-        memmove(wbuffer(), begin, len());
-    wbuffer()[len()] = 0;
+        memmove(wbuffer(), begin, newlen);
+    wbuffer()[newlen] = 0;
 }
 
 // /*********************************************/

cores/esp8266/WString.h

@@ -243,17 +243,20 @@ class String {
         float toFloat(void) const;
 
     protected:
+        // Contains the string info when we're not in SSO mode
+        struct _ptr { 
+            char *   buff;
+            uint16_t cap;
+            uint16_t len;
+        };
+
         // SSO is handled by checking the last byte of sso_buff.
         // When not in SSO mode, that byte is set to 0xff, while when in SSO mode it is always 0x00 (so it can serve as the string terminator as well as a flag)
         // This allows strings up up to 12 (11 + \0 termination) without any extra space.
-        enum { SSOSIZE = 12 }; // Characters to allocate space for SSO, must be 12 or more
+        enum { SSOSIZE = sizeof(struct _ptr) + 4 }; // Characters to allocate space for SSO, must be 12 or more
         enum { CAPACITY_MAX = 65535 }; // If size of capacity changed, be sure to update this enum
         union {
-            struct {
-                uint16_t cap;
-                uint16_t len;
-                char *   buf;
-            } ptr;
+            struct _ptr ptr;
             char sso_buf[SSOSIZE];
         };
         // Accessor functions
@@ -263,9 +266,10 @@ class String {
         inline void setSSO(bool sso) { sso_buf[SSOSIZE - 1] = sso ? 0x00 : 0xff; }
         inline void setLen(int len) { if (!sso()) ptr.len = len; }
         inline void setCapacity(int cap) { if (!sso()) ptr.cap = cap; }
+	inline void setBuffer(char *buff) { if (!sso()) ptr.buff = buff; }
         // Buffer accessor functions
-        inline const char *buffer() const { return (const char *)(sso() ? sso_buf : ptr.buf); }
-        inline char *wbuffer() const { return sso() ? const_cast<char *>(sso_buf) : ptr.buf; } // Writable version of buffer
+        inline const char *buffer() const { return (const char *)(sso() ? sso_buf : ptr.buff); }
+        inline char *wbuffer() const { return sso() ? const_cast<char *>(sso_buf) : ptr.buff; } // Writable version of buffer
 
     protected:
         void init(void);

tests/host/core/test_string.cpp

@@ -317,12 +317,47 @@ TEST_CASE("String SSO works", "[core][String]")
   REQUIRE(s.c_str() == savesso);
   REQUIRE(s == "0123456789a");
   REQUIRE(s.length() == 11);
-  s += "b";
-  REQUIRE(s.c_str() != savesso);
-  REQUIRE(s == "0123456789ab");
-  REQUIRE(s.length() == 12);
-  s += "c";
-  REQUIRE(s.c_str() != savesso);
-  REQUIRE(s == "0123456789abc");
-  REQUIRE(s.length() == 13);
+  if (sizeof(savesso) == 4) {
+    s += "b";
+    REQUIRE(s.c_str() != savesso);
+    REQUIRE(s == "0123456789ab");
+    REQUIRE(s.length() == 12);
+    s += "c";
+    REQUIRE(s.c_str() != savesso);
+    REQUIRE(s == "0123456789abc");
+    REQUIRE(s.length() == 13);
+  } else {
+    s += "bcde";
+    REQUIRE(s.c_str() == savesso);
+    REQUIRE(s == "0123456789abcde");
+    REQUIRE(s.length() == 15);
+    s += "fghi";
+    REQUIRE(s.c_str() == savesso);
+    REQUIRE(s == "0123456789abcdefghi");
+    REQUIRE(s.length() == 19);
+    s += "j";
+    REQUIRE(s.c_str() != savesso);
+    REQUIRE(s == "0123456789abcdefghij");
+    REQUIRE(s.length() == 20);
+    s += "k";
+    REQUIRE(s.c_str() != savesso);
+    REQUIRE(s == "0123456789abcdefghijk");
+    REQUIRE(s.length() == 21);
+    s += "l";
+    REQUIRE(s.c_str() != savesso);
+    REQUIRE(s == "0123456789abcdefghijkl");
+    REQUIRE(s.length() == 22);
+    s += "m";
+    REQUIRE(s.c_str() != savesso);
+    REQUIRE(s == "0123456789abcdefghijklm");
+    REQUIRE(s.length() == 23);
+    s += "nopq";
+    REQUIRE(s.c_str() != savesso);
+      REQUIRE(s == "0123456789abcdefghijklmnopq");
+    REQUIRE(s.length() == 27);
+    s += "rstu";
+    REQUIRE(s.c_str() != savesso);
+    REQUIRE(s == "0123456789abcdefghijklmnopqrstu");
+    REQUIRE(s.length() == 31);
+  }
 }