Skip to content

Commit

Permalink
common/json.c: Check that JSMN result is well-formed.
Browse files Browse the repository at this point in the history
xref: https://lists.ozlabs.org/pipermail/c-lightning/2020-June/000188.html

Changelog-Fixed: Reject some bad JSON at parsing.
  • Loading branch information
ZmnSCPxj committed Jun 10, 2020
1 parent 0f568e1 commit 469a2eb
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 4 deletions.
86 changes: 85 additions & 1 deletion common/json.c
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,90 @@ const jsmntok_t *json_get_arr(const jsmntok_t tok[], size_t index)
return NULL;
}

/* Finds the end of a JSMN token. */
static const jsmntok_t *
find_jsmn_bounds(const jsmntok_t *toks, const jsmntok_t *toks_end, int size)
{
const jsmntok_t *p = toks;
const jsmntok_t *e = toks_end;
int i;

for (i = 0; i < size && p < e; ++i) {
if (p->size == 0)
++p;
else
p = find_jsmn_bounds(p + 1, e, p->size);
}

return p;
}

/* Validate that the JSMN result is not weird, e.g.
{ "" "" "" }, which JSMN will accept incorectly.
*/
static bool
validate_jsmn_result(const jsmntok_t *toks, const jsmntok_t *toks_end)
{
const jsmntok_t *p = toks;
const jsmntok_t *e = toks_end;

while (p < e) {
if (p->type == JSMN_STRING
|| p->type == JSMN_PRIMITIVE
|| p->type == JSMN_UNDEFINED) {
/* Top-level, non-object-owned should have 0 size. */
if (p->size != 0)
return false;
++p;
} else if (p->type == JSMN_ARRAY) {
const jsmntok_t *arrayend;
arrayend = find_jsmn_bounds(p + 1, e, p->size);
/* Invalid array. */
if (p->size != 0 && arrayend == p + 1)
return false;
if (!validate_jsmn_result(p + 1, arrayend))
return false;
p = arrayend;
} else if (p->type == JSMN_OBJECT) {
const jsmntok_t *pp;
const jsmntok_t *objend;
int count = 0;
objend = find_jsmn_bounds(p + 1, e, p->size);

/* Invalid object. */
if (p->size != 0 && objend == p + 1)
return false;

pp = p + 1;
while (pp < objend) {
/* Check the key. */
if (pp->type != JSMN_STRING)
return false;
if (pp->size != 1)
return false;

const jsmntok_t *valueend;
valueend = find_jsmn_bounds(pp + 1, objend, pp->size);
if (pp->size != 0 && valueend == pp + 1)
return false;
if (!validate_jsmn_result(pp + 1, valueend))
return false;
pp = valueend;
++count;
}
if (count != p->size)
return false;

p = objend;
} else
/* Unknown type. */
return false;
}

return true;
}


jsmntok_t *json_parse_input(const tal_t *ctx,
const char *input, int len, bool *valid)
{
Expand Down Expand Up @@ -338,7 +422,7 @@ jsmntok_t *json_parse_input(const tal_t *ctx,
ret = json_next(toks) - toks;

/* Cut to length and return. */
*valid = true;
*valid = validate_jsmn_result(toks, toks + ret);
tal_resize(&toks, ret + 1);
/* Make sure last one is always referenceable. */
toks[ret].type = -1;
Expand Down
11 changes: 8 additions & 3 deletions common/test/run-json.c
Original file line number Diff line number Diff line change
Expand Up @@ -122,20 +122,25 @@ static void test_json_tok_size(void)
assert(toks[0].size == 2);
assert(toks[1].size == 2);

buf = "{\"e1\" : {\"e2\", \"e3\"}}";
buf = "{\"e1\" : {\"e2\": 2, \"e3\": 3}}";
toks = json_parse_input(tmpctx, buf, strlen(buf), &ok);
assert(ok);
/* size only counts *direct* children */
assert(toks[0].size == 1);
assert(toks[2].size == 2);

buf = "{\"e1\" : {\"e2\", \"e3\"}, \"e4\" : {\"e5\", \"e6\"}}";
buf = "{\"e1\" : {\"e2\": 2, \"e3\": 3}, \"e4\" : {\"e5\": 5, \"e6\": 6}}";
toks = json_parse_input(tmpctx, buf, strlen(buf), &ok);
assert(ok);
/* size only counts *direct* children */
assert(toks[0].size == 2);
assert(toks[2].size == 2);
assert(toks[6].size == 2);
assert(toks[8].size == 2);

/* This should *not* parse! (used to give toks[0]->size == 3!) */
buf = "{ \"\" \"\" \"\" }";
toks = json_parse_input(tmpctx, buf, strlen(buf), &ok);
assert(!ok);

/* This should *not* parse! (used to give toks[0]->size == 2!) */
buf = "{ 'satoshi', '546' }";
Expand Down

0 comments on commit 469a2eb

Please sign in to comment.