Skip to content

Commit

Permalink
Fix buffer overflow (pull request #81)
Browse files Browse the repository at this point in the history
  • Loading branch information
gcanalesb authored and bblanchon committed Jun 10, 2015
1 parent 08d05df commit 5e7b9ec
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 17 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
Arduino JSON: change log
========================

v4.5
----

* Fixed buffer overflow when input contains a backslash followed by a terminator (issue #81)

**Upgrading is recommended** since previous versions contain a potential security risk.

v4.4
----

Expand Down
32 changes: 15 additions & 17 deletions src/Internals/QuotedString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,46 +58,44 @@ static char unescapeChar(char c) {
static inline bool isQuote(char c) { return c == '\"' || c == '\''; }

char *QuotedString::extractFrom(char *input, char **endPtr) {
char firstChar = *input;

if (!isQuote(firstChar)) {
// must start with a quote
return NULL;
}

char stopChar = firstChar; // closing quote is the same as opening quote

char *startPtr = input + 1; // skip the quote
char *readPtr = startPtr;
char *writePtr = startPtr;
char c;

char firstChar = *input;
char stopChar = firstChar; // closing quote is the same as opening quote

if (!isQuote(firstChar)) goto ERROR_OPENING_QUOTE_MISSING;

for (;;) {
c = *readPtr++;

if (c == '\0') {
// premature ending
return NULL;
}
if (c == '\0') goto ERROR_CLOSING_QUOTE_MISSING;

if (c == stopChar) {
// closing quote
break;
}
if (c == stopChar) goto SUCCESS;

if (c == '\\') {
// replace char
c = unescapeChar(*readPtr++);
if (c == '\0') goto ERROR_ESCAPE_SEQUENCE_INTERRUPTED;
}

*writePtr++ = c;
}

SUCCESS:
// end the string here
*writePtr = '\0';

// update end ptr
*endPtr = readPtr;

// return pointer to unquoted string
return startPtr;

ERROR_OPENING_QUOTE_MISSING:
ERROR_CLOSING_QUOTE_MISSING:
ERROR_ESCAPE_SEQUENCE_INTERRUPTED:
return NULL;
}
10 changes: 10 additions & 0 deletions test/QuotedString_ExtractFrom_Tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ class QuotedString_ExtractFrom_Tests : public testing::Test {
_result = QuotedString::extractFrom(_jsonString, &_trailing);
}

void whenInputIs(const char *json, size_t len) {
memcpy(_jsonString, json, len);
_result = QuotedString::extractFrom(_jsonString, &_trailing);
}

void resultMustBe(const char *expected) { EXPECT_STREQ(expected, _result); }

void trailingMustBe(const char *expected) {
Expand Down Expand Up @@ -134,3 +139,8 @@ TEST_F(QuotedString_ExtractFrom_Tests, AllEscapedCharsTogether) {
whenInputIs("\"1\\\"2\\\\3\\/4\\b5\\f6\\n7\\r8\\t9\"");
resultMustBe("1\"2\\3/4\b5\f6\n7\r8\t9");
}

TEST_F(QuotedString_ExtractFrom_Tests, UnterminatedEscapeSequence) {
whenInputIs("\"\\\0\"", 4);
resultMustBe(0);
}

1 comment on commit 5e7b9ec

@fgeek
Copy link

@fgeek fgeek commented on 5e7b9ec Jun 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.