Skip to content

Commit ff74813

Browse files
earlephilhowerdevyte
authored andcommitted
Fix String creation and concat issues (esp8266#4955)
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.
1 parent bde83e8 commit ff74813

File tree

1 file changed

+29
-6
lines changed

1 file changed

+29
-6
lines changed

cores/esp8266/WString.cpp

+29-6
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,11 @@ String::String(unsigned char value, unsigned char base) {
7575
String::String(int value, unsigned char base) {
7676
init();
7777
char buf[2 + 8 * sizeof(int)];
78-
itoa(value, buf, base);
78+
if (base == 10) {
79+
sprintf(buf, "%d", value);
80+
} else {
81+
itoa(value, buf, base);
82+
}
7983
*this = buf;
8084
}
8185

@@ -89,7 +93,11 @@ String::String(unsigned int value, unsigned char base) {
8993
String::String(long value, unsigned char base) {
9094
init();
9195
char buf[2 + 8 * sizeof(long)];
92-
ltoa(value, buf, base);
96+
if (base==10) {
97+
sprintf(buf, "%ld", value);
98+
} else {
99+
ltoa(value, buf, base);
100+
}
93101
*this = buf;
94102
}
95103

@@ -252,7 +260,22 @@ String & String::operator = (const __FlashStringHelper *pstr)
252260
// /*********************************************/
253261

254262
unsigned char String::concat(const String &s) {
255-
return concat(s.buffer, s.len);
263+
// Special case if we're concatting ourself (s += s;) since we may end up
264+
// realloc'ing the buffer and moving s.buffer in the method called
265+
if (&s == this) {
266+
unsigned int newlen = 2 * len;
267+
if (!s.buffer)
268+
return 0;
269+
if (s.len == 0)
270+
return 1;
271+
if (!reserve(newlen))
272+
return 0;
273+
memcpy(s.buffer + len, s.buffer, len);
274+
len = newlen;
275+
return 1;
276+
} else {
277+
return concat(s.buffer, s.len);
278+
}
256279
}
257280

258281
unsigned char String::concat(const char *cstr, unsigned int length) {
@@ -283,13 +306,13 @@ unsigned char String::concat(char c) {
283306

284307
unsigned char String::concat(unsigned char num) {
285308
char buf[1 + 3 * sizeof(unsigned char)];
286-
itoa(num, buf, 10);
309+
sprintf(buf, "%d", num);
287310
return concat(buf, strlen(buf));
288311
}
289312

290313
unsigned char String::concat(int num) {
291314
char buf[2 + 3 * sizeof(int)];
292-
itoa(num, buf, 10);
315+
sprintf(buf, "%d", num);
293316
return concat(buf, strlen(buf));
294317
}
295318

@@ -301,7 +324,7 @@ unsigned char String::concat(unsigned int num) {
301324

302325
unsigned char String::concat(long num) {
303326
char buf[2 + 3 * sizeof(long)];
304-
ltoa(num, buf, 10);
327+
sprintf(buf, "%ld", num);
305328
return concat(buf, strlen(buf));
306329
}
307330

0 commit comments

Comments
 (0)