Skip to content

Commit

Permalink
Don't crash on invalid variable names in RPN
Browse files Browse the repository at this point in the history
When rpn_parse() finds [^\0,] after parsing a token, it returns NULL
without setting an error. This causes rrd_test_error() to return false
and subsequent code will dereference NULL (cf. rrdtool xport
CDEF:foo=foo-bar).

This commit changes the OP_VARIABLE branch in rpn_parse so that in order
to be a variable name, sscanf needs to match the full name, causing a
more meaningful "ERROR: don't understand 'illegal-variable-name'" error
message in that case. Also, I made the return NULL branch set an error
message so rrd_test_error() will succeed.
  • Loading branch information
yath committed Oct 19, 2015
1 parent 8b9f927 commit 0c3e8db
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/rrd_rpncalc.c
Original file line number Diff line number Diff line change
Expand Up @@ -433,14 +433,15 @@ rpnp_t *rpn_parse(

#undef match_op
else if ((sscanf(expr, DEF_NAM_FMT "%n", vname, &pos) == 1)
&& (expr[pos] == '\0' || expr[pos] == ',')
&& ((rpnp[steps].ptr = (*lookup) (key_hash, vname)) !=
-1)) {
rpnp[steps].op = OP_VARIABLE;
expr += pos;
}

else {
rrd_set_error("don't undestand '%s'",expr);
rrd_set_error("don't understand '%s'",expr);
free(rpnp);
return NULL;
}
Expand All @@ -453,6 +454,7 @@ rpnp_t *rpn_parse(
if (*expr == ',')
expr++;
else {
rrd_set_error("garbage in RPN: '%s'", expr);
free(rpnp);
return NULL;
}
Expand Down

0 comments on commit 0c3e8db

Please sign in to comment.