Unhandled error return value in sajson.h: parser::parse()

[cneckar]

Within the parser::parse() function of sajson.h the return value of a call to parse_string() is ignored leading to a corrupt representation of the parsed objects.

From sajson::parser::parse()

if (current_structure_type == TYPE_OBJECT && c != '}') {
  if (c != '"') {
    return error("object key must be quoted");
  }
  result = parse_string(temp);
  if (peek_structure() != ':') {
    return error("expected :");
  }
  ++p;
  temp += 2;
}

When parsing an object key string the return value result of parse_string() is later overwritten. As long as the character following the string parsing error is a ':' and the value for the key is valid, the parser will overwrite that error return value before it is checked and parsing will continue as if the key string is valid.

The bug can be demonstrated in a debugger using the following input to sajson::parser::parse()

{"\:0}

The initial object's key string is invalid and will generate an error within sajson::parser::parse_string_slow()

switch (*p) {
...
default:
  return error("unknown escape");

This error value will be passed back to sajson::parser::parse() and written to the result variable. However, upon entering the following code the result variable will be overwritten with the return value of sajson::parser::parse_number().

switch (peek_structure()) {
  type next_type;
  ...
  case '0':
  case '1':
  case '2':
  case '3':
  case '4':
  case '5':
  case '6':
  case '7':
  case '8':
  case '9':
  case '-':
    result = parse_number();
    break;
  ...
if (!result) {
  return result.success;
}

When the value of result is finally checked the error value will no longer be present and parsing will continue with a bad element. In this simple case the parser does not crash.

A more complicated object hierarchy can cause the parser to crash when the object containing the bad key is passed to sajson::parser::install_object(). The following input to sajson::parser::parse() can be used to demonstrate a crash when a corrupted object is sorted.

{"":0,"":0,"":[0.,""],"\:[0.,""],"\:[]}

The following patch can be implemented to correctly check the return value of sajson::parser::parse_string()

diff --git a/include/sajson.h b/include/sajson.h
index 350c58e..7ad08ad 100644
--- a/include/sajson.h
+++ b/include/sajson.h
@@ -618,6 +618,9 @@ namespace sajson {
                         return error("object key must be quoted");
                     }
                     result = parse_string(temp);
+                    if (!result) {
+                        return result.success;
+                    }
                     if (peek_structure() != ':') {
                         return error("expected :");
                     }

[chadaustin]

Excellent bug report! Thanks, the patch is checked in. Let me know if you see anything else.

[cneckar]

Will do, thanks for the quick turn around!

[cneckar]

FYI

MITRE assigned this bug CVE-2015-2299