Skip to content

Commit

Permalink
Fix String creation and concat issues (#4955)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
earlephilhower authored and devyte committed Jul 24, 2018
1 parent bde83e8 commit ff74813
Showing 1 changed file with 29 additions and 6 deletions.
35 changes: 29 additions & 6 deletions cores/esp8266/WString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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));
}

Expand All @@ -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));
}

Expand Down

0 comments on commit ff74813

Please sign in to comment.