From: Sebastian Schmidt <yath@yath.de>
Date: Sun, 18 Oct 2015 17:15:37 +0000 (+0200)
Subject: Don't crash on invalid variable names in RPN
X-Git-Tag: v1.6.0~25^2~8^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F677%2Fhead;p=thirdparty%2Frrdtool-1.x.git

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 ad951bfb..b39f2512 100644
--- 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;
         }