Created by: ghost
After seeing how useful this library has proven to be to within the Arduino community, I decided to fuzz the parser.
All the fuzzing and testing was done in a Linux x64 environment, using AFL and LLVM's AddressSanitizer in order to hunt for weird behavior.
Shortly after beginning to fuzz the parser, I encountered a crash with the following sequence:
00000000 7b 22 20 20 20 20 20 20 20 20 20 20 20 20 20 20 |{" |
00000010 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 | |
*
00000030 33 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 |3 |
00000040 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 | |
00000050 20 20 20 20 20 20 20 20 20 76 20 20 20 20 20 20 | v |
00000060 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 | |
*
00000080 20 33 20 20 20 20 20 20 20 20 20 20 20 20 20 20 | 3 |
00000090 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 | |
*
000000e0 20 20 20 4b 5c 5c 5c 00 | K\\\.|
000000e7
This interesting sequence produces a buffer over-read and overflow in the stack or heap, depending on where the JSON string is located, and can cause memory corruptions that crash the program.
The culprit is the escaped characters at the end of the quoted string, they cause the extractFrom
function to miss the NULL terminator on the next cycle.
The original extractFrom
code is as follows:
char *QuotedString::extractFrom(char *input, char **endPtr) {
...
for (;;) {
c = *readPtr++;
if (c == '\0') {
// premature ending
return NULL;
}
if (c == stopChar) {
// closing quote
break;
}
>>>
if (c == '\\') {
// replace char
c = unescapeChar(*readPtr++);
}
>>>
*writePtr++ = c;
}
...
}
As you can see, this operation will eventually lead the NULL check to miss the string's null terminator, effectively causing a buffer over-read and overflow which can lead to security issues.
In order to illustrate the issue, let []
denote the pointer position, let !
denote NULL, and let this be the pointer state:
"[\]\\!"
The unescapeChar
function will move the pointer to:
"\[\]\!"
At the beginning of the next cycle, the pointer will move one more step to:
"\\[\]!"
And the unescapeChar
function will once again move the pointer to:
"\\\[!]"
And, finally, at the beginning of the next cycle, the pointer will move and skip the null terminator:
"\\\![]"
The fix I propose simply checks to see if we've reached a premature ending after unescaping the character:
--- a/src/Internals/QuotedString.cpp
+++ b/src/Internals/QuotedString.cpp
@@ -73,6 +73,7 @@ char *QuotedString::extractFrom(char *input, char **endPtr) {
char c;
for (;;) {
c = *readPtr++;
if (c == '\0') {
@@ -90,6 +91,11 @@ char *QuotedString::extractFrom(char *input, char **endPtr) {
c = unescapeChar(*readPtr++);
}
+ if (c == '\0') {
+ // premature ending
+ return NULL;
+ }
+
*writePtr++ = c;
}
The ArduinoJson library passes all built-in tests with this fix.