Don't crash on invalid variable names in RPN

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.

diff --git a/src/rrd_rpncalc.c b/src/rrd_rpncalc.c
index ad951bfbb8374aeda89fa5adde0227e80d20583e..b39f251211b97df6185ae9b29ed97eadb0a2d19e 100644 (file)
--- a/src/rrd_rpncalc.c
+++ b/src/rrd_rpncalc.c
@@ -433,6 +433,7 @@ 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;
@@ -440,7 +441,7 @@ rpnp_t   *rpn_parse(
         }
 
         else {
-            rrd_set_error("don't undestand '%s'",expr);
+            rrd_set_error("don't understand '%s'",expr);
             free(rpnp);
             return NULL;
         }
@@ -453,6 +454,7 @@ rpnp_t   *rpn_parse(
         if (*expr == ',')
             expr++;
         else {
+            rrd_set_error("garbage in RPN: '%s'", expr);
             free(rpnp);
             return NULL;
         }