Invalid read when querying for a field that doesn't exist

[shreyasbharath]

Hi again,

I was writing some simple parsing tests and came across this error when I ran my test on Valgrind.

Given a test vector like this one -

static const char *jsonSimpleObject = R"*(
{
   "firstname": "first",
   "lastname": "last"
}
)*";

If I try to get a field that doesn't exist like so -

char *json = strclone( jsonSimpleObject );
JsonHashTable hashTable = parser.parseHashTable( json );

EXPECT_TRUE( hashTable.success() );
EXPECT_THAT( hashTable.getString( "firstname" ), StrEq( "first" ) );
EXPECT_THAT( hashTable.getString( "lastname" ), StrEq( "last" ) );
EXPECT_THAT( hashTable.getString( "middlename" ), Eq( nullptr ) );

The error reported by Valgrind is as below -

==16231== Conditional jump or move depends on uninitialised value(s)
==16231==    at 0x81983E5: JsonObjectBase::getNestedTokenCount(jsmntok_t*) (JsonObjectBase.cpp:18)
==16231==    by 0x8198253: JsonHashTable::getToken(char const*) (JsonHashTable.cpp:44)
==16231==    by 0x81983A5: JsonHashTable::getString(char const*) (JsonHashTable.cpp:83)
==16231==    by 0x80EF597: 

[bblanchon]

Valgrind says it's in JsonObjectBase.cpp:18 which is:

while (token->start < end)

It could be either: token, token->end or token->start that is not initialized.

Unfortunately, it's not easy for me to run Valgrind, because I'm using Visual Studio for the tests.
Maybe it's time for me to move to GCC and gtest...

[shreyasbharath]

I had a closer look and it appears that the "tokens" member variable in JsonParser.h is not initialised at all.

I think maybe it should be zeroed in the constructor or else it will be undefined when you try to read past the end of the last token.

[bblanchon]

Writing zeros in the array, just for the sake of it would be a complete waste.
But if there is really a bug, we should be able to write a unit test and fix it.

tokens should be initialized by jsmn_parse(), but maybe some of them are not.
Can you try with the latest version of jsmn?

Do you know which EXPECT_ line triggers this error?
I guess it could be the last one, because the key doesn't exists.

[shreyasbharath]

Yup it is the last EXPECT in that test.

I'll try out the latest version of Jsmn tomorrow (if we are not already using it).

[bblanchon]

Good!
I'm going to write a unit test.
There must be a sleeping bug here.

[shreyasbharath]

I can confirm that it is token->start that is uninitialised, it was a large negative number when I stepped through the code.

[bblanchon]

I refactored getNestedTokenCount().

I pushed a fix into the master branch, please give it a try.

[shreyasbharath]

I can confirm that this bug has been fixed. Thanks :)