Fix String creation and concat issues (#4955)

earlephilhower · devyte · commit ff74813d5422 · 2018-07-24T16:20:57.000-04:00
When a string is concatted to itself, the pointer to its c_str can change
due to realloc().  This would invalidate the passed-in pointer being
concatted, and cause a use-after-free error.  Special case this to avoid
the issue.  Now "a += a;" works properly.

Also use sprintf(%{l}d) instead of non-POSIX ltoa/itoa calls to construct a
string from a signed number (in base 10 only).  The non-posix versions don't
handle INT_MIN properly on either host_tests or on the ESP8266.
diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp
@@ -75,7 +75,11 @@ String::String(unsigned char value, unsigned char base) {
 String::String(int value, unsigned char base) {
     init();
     char buf[2 + 8 * sizeof(int)];
-    itoa(value, buf, base);
+    if (base == 10) {
+        sprintf(buf, "%d", value);
+    } else {
+        itoa(value, buf, base);
+    }
     *this = buf;
 }
 
@@ -89,7 +93,11 @@ String::String(unsigned int value, unsigned char base) {
 String::String(long value, unsigned char base) {
     init();
     char buf[2 + 8 * sizeof(long)];
-    ltoa(value, buf, base);
+    if (base==10) {
+        sprintf(buf, "%ld", value);
+    } else {
+        ltoa(value, buf, base);
+    }
     *this = buf;
 }
 
@@ -252,7 +260,22 @@ String & String::operator = (const __FlashStringHelper *pstr)
 // /*********************************************/
 
 unsigned char String::concat(const String &s) {
-    return concat(s.buffer, s.len);
+    // 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 0;
+        if (s.len == 0)
+            return 1;
+        if (!reserve(newlen))
+            return 0;
+        memcpy(s.buffer + len, s.buffer, len);
+        len = newlen;
+        return 1;
+    } else {
+        return concat(s.buffer, s.len);
+    }
 }
 
 unsigned char String::concat(const char *cstr, unsigned int length) {
@@ -283,13 +306,13 @@ unsigned char String::concat(char c) {
 
 unsigned char String::concat(unsigned char num) {
     char buf[1 + 3 * sizeof(unsigned char)];
-    itoa(num, buf, 10);
+    sprintf(buf, "%d", num);
     return concat(buf, strlen(buf));
 }
 
 unsigned char String::concat(int num) {
     char buf[2 + 3 * sizeof(int)];
-    itoa(num, buf, 10);
+    sprintf(buf, "%d", num);
     return concat(buf, strlen(buf));
 }
 
@@ -301,7 +324,7 @@ unsigned char String::concat(unsigned int num) {
 
 unsigned char String::concat(long num) {
     char buf[2 + 3 * sizeof(long)];
-    ltoa(num, buf, 10);
+    sprintf(buf, "%ld", num);
     return concat(buf, strlen(buf));
 }
 
