CVE-2018-18836: Improper Control of Generation of Code ('Code Injection') (CWE-94)
value: An issue was discovered in Netdata 1.10.0. JSON injection exists via the api/v1/data tqx parameter because of web_client_api_request_v1_data in web/api/web_api_v1.c.
CWE description: The product constructs all or part of a code segment using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the syntax or behavior of the intended code segment.
CVSS v3 base score: 6.5
Published: 2019-06-18T16:15Z
Repository: https://github.com/netdata/netdata
Language: C
Files changed: libnetdata/url/url.c, web/api/web_api_v1.c
Commit date: 2018-10-30 02:35:57 +0200
Commit message: fixed vulnerabilities identified by red4sec.com (#4521)


Diff: diff --git a/libnetdata/url/url.c b/libnetdata/url/url.c
index 8a96063a..07a9f806 100644
--- a/libnetdata/url/url.c
+++ b/libnetdata/url/url.c
@@ -58,11 +58,13 @@ char *url_decode_r(char *to, char *url, size_t size) {
          *e = &to[size - 1]; // destination end
 
     while(*s && d < e) {
         if(unlikely(*s == '%')) {
             if(likely(s[1] && s[2])) {
-                *d++ = from_hex(s[1]) << 4 | from_hex(s[2]);
+                char t = from_hex(s[1]) << 4 | from_hex(s[2]);
+                // avoid HTTP header injection
+                *d++ = (char)((isprint(t))? t : ' ');
                 s += 2;
             }
         }
         else if(unlikely(*s == '+'))
             *d++ = ' ';
diff --git a/web/api/web_api_v1.c b/web/api/web_api_v1.c
index 0c0242c3..1e03828e 100644
--- a/web/api/web_api_v1.c
+++ b/web/api/web_api_v1.c
@@ -231,10 +231,19 @@ inline int web_client_api_request_v1_charts(RRDHOST *host, struct web_client *w,
 
 inline int web_client_api_request_v1_chart(RRDHOST *host, struct web_client *w, char *url) {
     return web_client_api_request_single_chart(host, w, url, rrd_stats_api_v1_chart);
 }
 
+void fix_google_param(char *s) {
+    if(unlikely(!s)) return;
+
+    for( ; *s ;s++) {
+        if(!isalnum(*s) && *s != '.' && *s != '_' && *s != '-')
+            *s = '_';
+    }
+}
+
 // returns the HTTP code
 inline int web_client_api_request_v1_data(RRDHOST *host, struct web_client *w, char *url) {
     debug(D_WEB_CLIENT, "%llu: API v1 data with URL '%s'", w->id, url);
 
     int ret = 400;
@@ -330,10 +339,18 @@ inline int web_client_api_request_v1_data(RRDHOST *host, struct web_client *w, c
                     outFileName = tqx_value;
             }
         }
     }
 
+    // validate the google parameters given
+    fix_google_param(google_out);
+    fix_google_param(google_sig);
+    fix_google_param(google_reqId);
+    fix_google_param(google_version);
+    fix_google_param(responseHandler);
+    fix_google_param(outFileName);
+
     if(!chart || !*chart) {
         buffer_sprintf(w->response.data, "No chart id is given at the request.");
         goto cleanup;
     }
 

Vulnerable code:                 *d++ = from_hex(s[1]) << 4 | from_hex(s[2]);
Fixed code:                 char t = from_hex(s[1]) << 4 | from_hex(s[2]);
                // avoid HTTP header injection
                *d++ = (char)((isprint(t))? t : ' ');
void fix_google_param(char *s) {
    if(unlikely(!s)) return;

    for( ; *s ;s++) {
        if(!isalnum(*s) && *s != '.' && *s != '_' && *s != '-')
            *s = '_';
    }
}

    // validate the google parameters given
    fix_google_param(google_out);
    fix_google_param(google_sig);
    fix_google_param(google_reqId);
    fix_google_param(google_version);
    fix_google_param(responseHandler);
    fix_google_param(outFileName);