CVE-2014-6262: Use of Externally-Controlled Format String (CWE-134)
value: Multiple format string vulnerabilities in the python module in RRDtool, as used in Zenoss Core before 4.2.5 and other products, allow remote attackers to execute arbitrary code or cause a denial of service (application crash) via a crafted third argument to the rrdtool.graph function, aka ZEN-15415, a related issue to CVE-2013-2131.
CWE description: The product uses a function that accepts a format string as an argument, but the format string originates from an external source.
CVSS v3 base score: 7.5
Published: 2020-02-12T02:15Z
Repository: https://github.com/oetiker/rrdtool-1.x
Language: C
Files changed: src/rrd_graph.c, src/rrd_graph.h
Commit date: 2014-09-16 09:48:26 +0200
Commit message: a proper fix to the bad_format checking misery

Conflicts:

	src/rrd_graph.c

Diff: diff --git a/src/rrd_graph.c b/src/rrd_graph.c
index f9f4d35..31de418 100644
--- a/src/rrd_graph.c
+++ b/src/rrd_graph.c
@@ -4,10 +4,11 @@
  * rrd__graph.c  produce graphs from data in rrdfiles
  ****************************************************************************/
 
 
 #include <sys/stat.h>
+#include <glib.h>   // will use regex
 
 #ifdef WIN32
 #include "strftime.h"
 #endif
 
@@ -1672,13 +1673,11 @@ int print_calc(
                        time_clean(prline.u_str, im->gdes[i].format);
                     } else {
                         strftime(prline.u_str,
                                  FMT_LEG_LEN, im->gdes[i].format, &tmvdef);
                     }
-                } else if (bad_format(im->gdes[i].format)) {
-                    rrd_set_error
-                        ("bad format for PRINT in '%s'", im->gdes[i].format);
+                } else if (bad_format_print(im->gdes[i].format)) {
                     return -1;
                 } else {
                     prline.u_str =
                         sprintf_alloc(im->gdes[i].format, printval, si_symb);
                 }
@@ -1695,15 +1694,12 @@ int print_calc(
                     } else {
                         strftime(im->gdes[i].legend,
                                  FMT_LEG_LEN, im->gdes[i].format, &tmvdef);
                     }
                 } else {
-                    if (bad_format(im->gdes[i].format)) {
-                        rrd_set_error
-                            ("bad format for GPRINT in '%s'",
-                             im->gdes[i].format);
-                        return -1;
+                    if (bad_format_print(im->gdes[i].format)) {
+                       return -1;
                     }
                     snprintf(im->gdes[i].legend,
                              FMT_LEG_LEN - 2,
                              im->gdes[i].format, printval, si_symb);
                 }
@@ -4170,11 +4166,10 @@ int rrd_graph(
     }
     rrd_info_free(grinfo);
     return 0;
 }
 
-
 /* Some surgery done on this function, it became ridiculously big.
 ** Things moved:
 ** - initializing     now in rrd_graph_init()
 ** - options parsing  now in rrd_graph_options()
 ** - script parsing   now in rrd_graph_script()
@@ -4241,11 +4236,10 @@ rrd_info_t *rrd_graph_v(
         char     *filename;
 
         if (bad_format_imginfo(im.imginfo)) {
             rrd_info_free(im.grinfo);
             im_free(&im);
-            rrd_set_error("bad format for imginfo");
             return NULL;
         }
         path = strdup(im.graphfile);
         filename = basename(path);
         info.u_str =
@@ -4715,23 +4709,21 @@ void rrd_graph_options(
                 rrd_set_error("cannot allocate memory for second_axis_legend");
                 return;
             }
             break;
         case 1004:
-            if (bad_format(optarg)){
-                rrd_set_error("use either %le or %lf formats");
+            if (bad_format_axis(optarg)){
                 return;
             }
             im->second_axis_format=strdup(optarg);
             if (!im->second_axis_format) {
                 rrd_set_error("cannot allocate memory for second_axis_format");
                 return;
             }
             break;
         case 1012:
-            if (bad_format(optarg)){
-                rrd_set_error("use either %le or %lf formats");
+            if (bad_format_axis(optarg)){
                 return;
             }
             im->primary_axis_format=strdup(optarg);
             if (!im->primary_axis_format) {
                 rrd_set_error("cannot allocate memory for primary_axis_format");
@@ -5065,103 +5057,42 @@ int rrd_graph_color(
         return n;
     }
 }
 
 
-int bad_format(
-    char *fmt)
-{
-    char     *ptr;
-    int       n = 0;
-
-    ptr = fmt;
-    while (*ptr != '\0')
-        if (*ptr++ == '%') {
-
-            /* line cannot end with percent char */
-            if (*ptr == '\0')
-                return 1;
-            /* '%s', '%S' and '%%' are allowed */
-            if (*ptr == 's' || *ptr == 'S' || *ptr == '%')
-                ptr++;
-            /* %c is allowed (but use only with vdef!) */
-            else if (*ptr == 'c') {
-                ptr++;
-                n = 1;
-            }
-
-            /* or else '% 6.2lf' and such are allowed */
-            else {
-                /* optional padding character */
-                if (*ptr == ' ' || *ptr == '+' || *ptr == '-')
-                    ptr++;
-                /* This should take care of 'm.n' with all three optional */
-                while (*ptr >= '0' && *ptr <= '9')
-                    ptr++;
-                if (*ptr == '.')
-                    ptr++;
-                while (*ptr >= '0' && *ptr <= '9')
-                    ptr++;
-                /* Either 'le', 'lf' or 'lg' must follow here */
-                if (*ptr++ != 'l')
-                    return 1;
-                if (*ptr == 'e' || *ptr == 'f' || *ptr == 'g')
-                    ptr++;
-                else
-                    return 1;
-                n++;
-            }
-        }
-
-    return (n != 1);
+static int bad_format_check(const char *pattern, char *fmt) {
+    GError *gerr = NULL;
+    GRegex *re = g_regex_new(pattern, G_REGEX_EXTENDED, 0, &gerr);
+    GMatchInfo *mi;
+    if (gerr != NULL) {
+        rrd_set_error("cannot compile regular expression: %s (%s)", gerr->message,pattern);
+        return 1;
+    }
+    int m = g_regex_match(re, fmt, 0, &mi);
+    g_match_info_free (mi);
+    g_regex_unref(re);
+    if (!m) {
+        rrd_set_error("invalid format string '%s' (should match '%s')",fmt,pattern);
+        return 1;
+    }
+    return 0;
 }
 
+#define SAFE_STRING "(?:[^%]+|%%)*"
 
-int bad_format_imginfo(
-    char *fmt)
-{
-    char     *ptr;
-    int       n = 0;
-
-    ptr = fmt;
-    while (*ptr != '\0')
-        if (*ptr++ == '%') {
-
-            /* line cannot end with percent char */
-            if (*ptr == '\0')
-                return 1;
-            /* '%%' is allowed */
-            if (*ptr == '%')
-                ptr++;
-            /* '%s', '%S' are allowed */
-            else if (*ptr == 's' || *ptr == 'S') {
-                n = 1;
-                ptr++;
-            }
-
-            /* or else '% 4lu' and such are allowed */
-            else {
-                /* optional padding character */
-                if (*ptr == ' ')
-                    ptr++;
-                /* This should take care of 'm' */
-                while (*ptr >= '0' && *ptr <= '9')
-                    ptr++;
-                /* 'lu' must follow here */
-                if (*ptr++ != 'l')
-                    return 1;
-                if (*ptr == 'u')
-                    ptr++;
-                else
-                    return 1;
-                n++;
-            }
-        }
+int bad_format_imginfo(char *fmt){
+    return bad_format_check("^" SAFE_STRING "%s" SAFE_STRING "%lu" SAFE_STRING "%lu" SAFE_STRING "$",fmt);
+}
+#define FLOAT_STRING "%[+- 0#]?[0-9]*([.][0-9]+)?l[eEfF]"
 
-    return (n != 3);
+int bad_format_axis(char *fmt){
+    return bad_format_check("^" SAFE_STRING FLOAT_STRING SAFE_STRING "$",fmt);
 }
 
+int bad_format_print(char *fmt){
+    return bad_format_check("^" SAFE_STRING FLOAT_STRING SAFE_STRING "%s" SAFE_STRING "$",fmt);
+}
 
 int vdef_parse(
     struct graph_desc_t
     *gdes,
     const char *const str)
diff --git a/src/rrd_graph.h b/src/rrd_graph.h
index 1093810..d0df273 100644
--- a/src/rrd_graph.h
+++ b/src/rrd_graph.h
@@ -451,11 +451,15 @@ void      rrd_graph_script(
 int       rrd_graph_color(
     image_desc_t *,
     char *,
     char *,
     int);
-int       bad_format(
+int       bad_format_axis(
+    char *);
+int       bad_format_print(
+    char *);
+int       bad_format_imginfo(
     char *);
 int       bad_format_imginfo(
     char *);
 int       vdef_parse(
     struct graph_desc_t *,

Vulnerable code:                 } else if (bad_format(im->gdes[i].format)) {
                    rrd_set_error
                        ("bad format for PRINT in '%s'", im->gdes[i].format);
                    if (bad_format(im->gdes[i].format)) {
                        rrd_set_error
                            ("bad format for GPRINT in '%s'",
                             im->gdes[i].format);
                        return -1;

            rrd_set_error("bad format for imginfo");
            if (bad_format(optarg)){
                rrd_set_error("use either %le or %lf formats");
            if (bad_format(optarg)){
                rrd_set_error("use either %le or %lf formats");
int bad_format(
    char *fmt)
{
    char     *ptr;
    int       n = 0;

    ptr = fmt;
    while (*ptr != '\0')
        if (*ptr++ == '%') {

            /* line cannot end with percent char */
            if (*ptr == '\0')
                return 1;
            /* '%s', '%S' and '%%' are allowed */
            if (*ptr == 's' || *ptr == 'S' || *ptr == '%')
                ptr++;
            /* %c is allowed (but use only with vdef!) */
            else if (*ptr == 'c') {
                ptr++;
                n = 1;
            }

            /* or else '% 6.2lf' and such are allowed */
            else {
                /* optional padding character */
                if (*ptr == ' ' || *ptr == '+' || *ptr == '-')
                    ptr++;
                /* This should take care of 'm.n' with all three optional */
                while (*ptr >= '0' && *ptr <= '9')
                    ptr++;
                if (*ptr == '.')
                    ptr++;
                while (*ptr >= '0' && *ptr <= '9')
                    ptr++;
                /* Either 'le', 'lf' or 'lg' must follow here */
                if (*ptr++ != 'l')
                    return 1;
                if (*ptr == 'e' || *ptr == 'f' || *ptr == 'g')
                    ptr++;
                else
                    return 1;
                n++;
            }
        }

    return (n != 1);
int bad_format_imginfo(
    char *fmt)
{
    char     *ptr;
    int       n = 0;

    ptr = fmt;
    while (*ptr != '\0')
        if (*ptr++ == '%') {

            /* line cannot end with percent char */
            if (*ptr == '\0')
                return 1;
            /* '%%' is allowed */
            if (*ptr == '%')
                ptr++;
            /* '%s', '%S' are allowed */
            else if (*ptr == 's' || *ptr == 'S') {
                n = 1;
                ptr++;
            }

            /* or else '% 4lu' and such are allowed */
            else {
                /* optional padding character */
                if (*ptr == ' ')
                    ptr++;
                /* This should take care of 'm' */
                while (*ptr >= '0' && *ptr <= '9')
                    ptr++;
                /* 'lu' must follow here */
                if (*ptr++ != 'l')
                    return 1;
                if (*ptr == 'u')
                    ptr++;
                else
                    return 1;
                n++;
            }
        }
    return (n != 3);
int       bad_format(
Fixed code: #include <glib.h>   // will use regex
                } else if (bad_format_print(im->gdes[i].format)) {
                    if (bad_format_print(im->gdes[i].format)) {
                       return -1;
            if (bad_format_axis(optarg)){
            if (bad_format_axis(optarg)){
static int bad_format_check(const char *pattern, char *fmt) {
    GError *gerr = NULL;
    GRegex *re = g_regex_new(pattern, G_REGEX_EXTENDED, 0, &gerr);
    GMatchInfo *mi;
    if (gerr != NULL) {
        rrd_set_error("cannot compile regular expression: %s (%s)", gerr->message,pattern);
        return 1;
    }
    int m = g_regex_match(re, fmt, 0, &mi);
    g_match_info_free (mi);
    g_regex_unref(re);
    if (!m) {
        rrd_set_error("invalid format string '%s' (should match '%s')",fmt,pattern);
        return 1;
    }
    return 0;
#define SAFE_STRING "(?:[^%]+|%%)*"
int bad_format_imginfo(char *fmt){
    return bad_format_check("^" SAFE_STRING "%s" SAFE_STRING "%lu" SAFE_STRING "%lu" SAFE_STRING "$",fmt);
}
#define FLOAT_STRING "%[+- 0#]?[0-9]*([.][0-9]+)?l[eEfF]"
int bad_format_axis(char *fmt){
    return bad_format_check("^" SAFE_STRING FLOAT_STRING SAFE_STRING "$",fmt);
int bad_format_print(char *fmt){
    return bad_format_check("^" SAFE_STRING FLOAT_STRING SAFE_STRING "%s" SAFE_STRING "$",fmt);
}
int       bad_format_axis(
    char *);
int       bad_format_print(
    char *);
int       bad_format_imginfo(