CVE-2018-18837: Improper Neutralization of CRLF Sequences in HTTP Headers ('HTTP Request/Response Splitting') (CWE-113)
value: An issue was discovered in Netdata 1.10.0. HTTP Header Injection exists via the api/v1/data filename parameter because of web_client_api_request_v1_data in web/api/web_api_v1.c.
CWE description: The product receives data from an HTTP agent/component (e.g., web server, proxy, browser, etc.), but it does not neutralize or incorrectly neutralizes CR and LF characters before the data is included in outgoing HTTP headers.
CVSS v3 base score: 6.1
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);