Skip to content

Stack smash with String(float, precision) #5873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MitchBradley opened this issue Nov 10, 2021 · 3 comments · Fixed by #6138
Closed

Stack smash with String(float, precision) #5873

MitchBradley opened this issue Nov 10, 2021 · 3 comments · Fixed by #6138
Assignees
Labels
Status: Awaiting triage Issue is waiting for triage

Comments

@MitchBradley
Copy link
Contributor

MitchBradley commented Nov 10, 2021

Passing a large floating point value, or a large precision, to String::String(float value, unsigned char decimalPlaces) causes a stack smash.

Any one of these lines, individually, in any context, will do it:

    String s = String(1e31f, 0);
    String s = String(-1.0f, 30);
    String s = String(0.0f, 31);

The problem is in WString.cpp which uses a buffer of length 33 char buf[33] Since FLT_MAX is 3.4e38, 33 bytes is not enough. Furthermore, there is no limit on the precision value that can be passed through to dtostrf(), so you can crash it either with large floating point values or large precision numbers.

To fix it without going into dtostrf(), you would have to both increase the buffer size to around 80 - accounting for possible minus sign, decimal point, 39 predecimal digits, 38 postdecimal digits, and null terminator, and also limit the number of decimal places to 38.

The problem is even worse with the version of String whose argument is double; there the buffer size would need to be about 620.

@SuGlider
Copy link
Collaborator

@MitchBradley
In order to better assist you, could you please run this code in order to get more information about your board and development environment.

What Arduino core are you using (1.0.6, 2.0.0 or 2.0.1)?
What IDE are you using (Arduino IDE, PlatforIO, etc)?

  Serial.printf("Internal Total heap %d, internal Free Heap %d\n", ESP.getHeapSize(), ESP.getFreeHeap());
  Serial.printf("SPIRam Total heap %d, SPIRam Free Heap %d\n", ESP.getPsramSize(), ESP.getFreePsram());
  Serial.printf("ChipRevision %d, Cpu Freq %d, SDK Version %s\n", ESP.getChipRevision(), ESP.getCpuFreqMHz(), ESP.getSdkVersion());
  Serial.printf("Flash Size %d, Flash Speed %d\n", ESP.getFlashChipSize(), ESP.getFlashChipSpeed());

@SuGlider SuGlider self-assigned this Nov 12, 2021
@MitchBradley
Copy link
Contributor Author

I have tried it with numerous setups - Arduino IDE, with Arduino core 1.0.6 and 2.0.1, and PlatformIO with framework-arduinoespressif32 version 3.10006.210326 and also a pull from git as of about a month ago.

git blame says that the code in question - the 33-character buffer - has not changed since it was imported from the esp8266 version 5 years ago. That suggests that the bug might be in the 8266 version too. Yes, same bug here

Here are two of the many environments I have tried:

Arduino IDE 1.8.13 + Arduino core 1.0.6

Internal Total heap 402508, internal Free Heap 371144
SPIRam Total heap 0, SPIRam Free Heap 0
ChipRevision 3, Cpu Freq 240, SDK Version v3.3.5-1-g85c43024c
Flash Size 4194304, Flash Speed 80000000
Stack smashing protect failure!

abort() was called at PC 0x400e2818 on core 1

ELF file SHA256: 0000000000000000

Backtrace: 0x40084e68:0x3ffb1eb0 0x400850dd:0x3ffb1ed0 0x400e2818:0x3ffb1ef0 0x400d18a5:0x3ffb1f10 0x400d0caf:0x3ffb1f70 0x400d196e:0x3ffb1fb0 0x400860ed:0x3ffb1fd0

Rebooting...

Arduino IDE 1.8.13 + Arduino ESP32 core 2.0.1

Internal Total heap 371232, internal Free Heap 346368
SPIRam Total heap 0, SPIRam Free Heap 0
ChipRevision 3, Cpu Freq 240, SDK Version v4.4-dev-3569-g6a7d83af19-dirty
Flash Size 4194304, Flash Speed 80000000

Stack smashing protect failure!

Here is the sketch:

void setup() {
  Serial.begin(115200);
  Serial.printf("Internal Total heap %d, internal Free Heap %d\n", ESP.getHeapSize(), ESP.getFreeHeap());
  Serial.printf("SPIRam Total heap %d, SPIRam Free Heap %d\n", ESP.getPsramSize(), ESP.getFreePsram());
  Serial.printf("ChipRevision %d, Cpu Freq %d, SDK Version %s\n", ESP.getChipRevision(), ESP.getCpuFreqMHz(), ESP.getSdkVersion());
  Serial.printf("Flash Size %d, Flash Speed %d\n", ESP.getFlashChipSize(), ESP.getFlashChipSpeed());
  delay(10000);
  String s = String(1e32f, 0);
}

void loop() {
  // put your main code here, to run repeatedly:
}

The delay(10000); slows down the reboot loop so the output can be captured.

@SuGlider
Copy link
Collaborator

@MitchBradley
Thanks again for reporting the issue.
There is a PR (#6138) that fixes the problem now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting triage Issue is waiting for triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants