-
Notifications
You must be signed in to change notification settings - Fork 654
Description
Describe the bug
in OLEDDisplay.cpp there is a long existing bug on handling the malloc failure.
To Reproduce
Found from code inspection.
In OLEDDisplay.cpp OLEDDisplay::allocateBuffer(), there is this code:
esp8266-oled-ssd1306/src/OLEDDisplay.cpp
Lines 73 to 81 in f90368e
if(this->buffer==NULL) { | |
this->buffer = (uint8_t*) malloc((sizeof(uint8_t) * displayBufferSize) + BufferOffset); | |
this->buffer += BufferOffset; | |
if(!this->buffer) { | |
DEBUG_OLEDDISPLAY("[OLEDDISPLAY][init] Not enough memory to create display\n"); | |
return false; | |
} | |
} |
In case of error, malloc returns 0. Adding BufferOffset before testing for 0 will fail always, and code will run with null pointer plus offset.
There is a near-identical part for buffer_back
with same error.
The construction if(!this->buffer)
does kind of hide the fact that it tests for NULL, which probably made the person inserting the addition above that less weary of that sideeffect.
Writing the test as if (this->buffer == NULL)
could have made the difference.
Expected behavior
The proper handling of malloc errors, should prevent null pointer(-based) operations.
Versions (please complete the following information):
- Library: versions 4.1.0 ... 4.6.1
- Platform: any
- This bug was introduced in release version 4.1.0 (5 Jun 2019) with 8648131