Description
In certain situations using the result of the String::substring() method will (read) deference NULL.
On some architectures this crashes the processor (Notable Teensy4)
[ using Arduino 1.8.13 / Teensyduino 1.53 - but the code in WString.cpp doesn't seem to have changed
for a long time ]
Necessary conditions:
substring() returns a null String
an existing non-null String is overwritten by this result.
The fault is that the String::move method in these circumstances calls strcpy with the source argument
as NULL.
You can see the bug report on the Teensy forum here: https://forum.pjrc.com/threads/67275-Issue-with-returning-zero-length-string-from-String-substring()
Example code to reproduce on Teensy4 (and presumably other architectures where address 0 faults if
deferenced):
void setup()
{
Serial.begin(115200) ;
delay(500) ;
Serial.println ("Testing:") ;
String a = "ABCDEF" ;
Serial.print ("1: '") ; Serial.print (a) ;
Serial.println ("'") ;
String b = a.substring(2, 2) ;
Serial.print ("2: '") ; Serial.print (b) ;
Serial.println ("'") ;
b = a.substring(2, 2) ;
Serial.print ("3: '") ; Serial.print (b) ;
Serial.println ("'") ; delay(10) ;
b = "1234" ;
b = a.substring(2, 2) ;
Serial.print ("4: '") ; Serial.print (b) ;
Serial.println ("'") ;
}
void loop() {}
Expected result:
Testing:
1: 'ABCDEF'
2: ''
3: ''
4: ''
What happens instead:
Testing:
1: 'ABCDEF'
2: ''
3: ''
and the system hangs/crashes.
The definition of String::move I have:
void String::move(String &rhs)
{
if (buffer) {
if (capacity >= rhs.len) {
strcpy(buffer, rhs.buffer);
len = rhs.len;
rhs.len = 0;
return;
} else {
free(buffer);
}
}
buffer = rhs.buffer;
capacity = rhs.capacity;
len = rhs.len;
rhs.buffer = NULL;
rhs.capacity = 0;
rhs.len = 0;
}
Suggested fix:
void String::move(String &rhs)
{
if (buffer) {
if (capacity >= rhs.len) {
if (rhs.buffer)
strcpy(buffer, rhs.buffer);
else
*buffer = '\0';
len = rhs.len;
rhs.len = 0;
return;
} else {
free(buffer);
}
}
buffer = rhs.buffer;
capacity = rhs.capacity;
len = rhs.len;
rhs.buffer = NULL;
rhs.capacity = 0;
rhs.len = 0;
}