CVE-2018-18838: Improper Encoding or Escaping of Output (CWE-116)
value: An issue was discovered in Netdata 1.10.0. Log Injection (or Log Forgery) exists via a %0a sequence in the url parameter to api/v1/registry.
CWE description: The product prepares a structured message for communication with another component, but encoding or escaping of the data is either missing or done incorrectly. As a result, the intended structure of the message is not preserved.
CVSS v3 base score: 7.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);