XSS fix: HTML Escape Before Inserting Untrusted Data into HTML Element Content; fixes...

diff --git a/src/web_buffer.c b/src/web_buffer.c
index 93ba782af279a05356765964862bdff770b07d4b..bc05f0d8d32f59f8aea65c69e7f628a51238e532 100644 (file)
--- a/src/web_buffer.c
+++ b/src/web_buffer.c
@@ -143,6 +143,26 @@ void buffer_strcat(BUFFER *wb, const char *txt)
     }
 }
 
+void buffer_strcat_htmlescape(BUFFER *wb, const char *txt)
+{
+    char b[2] = { [0] = '\0', [1] = '\0' };
+
+    while(*txt) {
+        switch(*txt) {
+            case '&': buffer_strcat(wb, "&"); break;
+            case '<': buffer_strcat(wb, "&lt;"); break;
+            case '>': buffer_strcat(wb, "&gt;"); break;
+            case '"': buffer_strcat(wb, "&quot;"); break;
+            case '/': buffer_strcat(wb, "&#x2F;"); break;
+            case '\'': buffer_strcat(wb, "&#x27;"); break;
+            default: {
+                b[0] = *txt;
+                buffer_strcat(wb, b);
+            }
+        }
+        txt++;
+    }
+}
 
 void buffer_snprintf(BUFFER *wb, size_t len, const char *fmt, ...)
 {

diff --git a/src/web_buffer.h b/src/web_buffer.h
index ee611209bc888c2fb24e7a4f4fb68d405b8849c1..4c2d599996ab99403ca7625b32cae88a99bbb061 100644 (file)
--- a/src/web_buffer.h
+++ b/src/web_buffer.h
@@ -64,6 +64,7 @@ extern void buffer_increase(BUFFER *b, size_t free_size_required);
 extern void buffer_snprintf(BUFFER *wb, size_t len, const char *fmt, ...) __attribute__ (( format (printf, 3, 4)));
 extern void buffer_vsprintf(BUFFER *wb, const char *fmt, va_list args);
 extern void buffer_sprintf(BUFFER *wb, const char *fmt, ...) __attribute__ (( format (printf, 2, 3)));
+extern void buffer_strcat_htmlescape(BUFFER *wb, const char *txt);
 
 extern void buffer_char_replace(BUFFER *wb, char from, char to);
 

diff --git a/src/web_client.c b/src/web_client.c
index 0280e142caccfd2dc64d75cf076bbd1ec17c1882..024533df112e1fce27e984b187ed4261c135a0c2 100644 (file)
--- a/src/web_client.c
+++ b/src/web_client.c
@@ -308,7 +308,8 @@ int mysendfile(struct web_client *w, char *filename)
     for(s = filename; *s ;s++) {
         if( !isalnum(*s) && *s != '/' && *s != '.' && *s != '-' && *s != '_') {
             debug(D_WEB_CLIENT_ACCESS, "%llu: File '%s' is not acceptable.", w->id, filename);
-            buffer_sprintf(w->response.data, "File '%s' cannot be served. Filename contains invalid character '%c'", filename, *s);
+            buffer_sprintf(w->response.data, "Filename contains invalid characters: ");
+            buffer_strcat_htmlescape(w->response.data, filename);
             return 400;
         }
     }
@@ -316,7 +317,8 @@ int mysendfile(struct web_client *w, char *filename)
     // if the filename contains a .. refuse to serve it
     if(strstr(filename, "..") != 0) {
         debug(D_WEB_CLIENT_ACCESS, "%llu: File '%s' is not acceptable.", w->id, filename);
-        buffer_sprintf(w->response.data, "File '%s' cannot be served. Relative filenames with '..' in them are not supported.", filename);
+        buffer_strcat(w->response.data, "Relative filenames are not supported: ");
+        buffer_strcat_htmlescape(w->response.data, filename);
         return 400;
     }
 
@@ -328,21 +330,24 @@ int mysendfile(struct web_client *w, char *filename)
     struct stat stat;
     if(lstat(webfilename, &stat) != 0) {
         debug(D_WEB_CLIENT_ACCESS, "%llu: File '%s' is not found.", w->id, webfilename);
-        buffer_sprintf(w->response.data, "File '%s' does not exist, or is not accessible.", webfilename);
+        buffer_strcat(w->response.data, "File does not exist, or is not accessible: ");
+        buffer_strcat_htmlescape(w->response.data, webfilename);
         return 404;
     }
 
     // check if the file is owned by expected user
     if(stat.st_uid != web_files_uid()) {
         error("%llu: File '%s' is owned by user %u (expected user %u). Access Denied.", w->id, webfilename, stat.st_uid, web_files_uid());
-        buffer_sprintf(w->response.data, "Access to file '%s' is not permitted.", webfilename);
+        buffer_strcat(w->response.data, "Access to file is not permitted: ");
+        buffer_strcat_htmlescape(w->response.data, webfilename);
         return 403;
     }
 
     // check if the file is owned by expected group
     if(stat.st_gid != web_files_gid()) {
         error("%llu: File '%s' is owned by group %u (expected group %u). Access Denied.", w->id, webfilename, stat.st_gid, web_files_gid());
-        buffer_sprintf(w->response.data, "Access to file '%s' is not permitted.", webfilename);
+        buffer_strcat(w->response.data, "Access to file is not permitted: ");
+        buffer_strcat_htmlescape(w->response.data, webfilename);
         return 403;
     }
 
@@ -353,7 +358,8 @@ int mysendfile(struct web_client *w, char *filename)
 
     if((stat.st_mode & S_IFMT) != S_IFREG) {
         error("%llu: File '%s' is not a regular file. Access Denied.", w->id, webfilename);
-        buffer_sprintf(w->response.data, "Access to file '%s' is not permitted.", webfilename);
+        buffer_strcat(w->response.data, "Access to file is not permitted: ");
+        buffer_strcat_htmlescape(w->response.data, webfilename);
         return 403;
     }
 
@@ -365,12 +371,14 @@ int mysendfile(struct web_client *w, char *filename)
         if(errno == EBUSY || errno == EAGAIN) {
             error("%llu: File '%s' is busy, sending 307 Moved Temporarily to force retry.", w->id, webfilename);
             buffer_sprintf(w->response.header, "Location: /" WEB_PATH_FILE "/%s\r\n", filename);
-            buffer_sprintf(w->response.data, "The file '%s' is currently busy. Please try again later.", webfilename);
+            buffer_strcat(w->response.data, "File is currently busy, please try again later: ");
+            buffer_strcat_htmlescape(w->response.data, webfilename);
             return 307;
         }
         else {
             error("%llu: Cannot open file '%s'.", w->id, webfilename);
-            buffer_sprintf(w->response.data, "Cannot open file '%s'.", webfilename);
+            buffer_strcat(w->response.data, "Cannot open file: ");
+            buffer_strcat_htmlescape(w->response.data, webfilename);
             return 404;
         }
     }
@@ -733,7 +741,8 @@ int web_client_api_request_single_chart(struct web_client *w, char *url, void ca
     RRDSET *st = rrdset_find(chart);
     if(!st) st = rrdset_find_byname(chart);
     if(!st) {
-        buffer_sprintf(w->response.data, "Chart '%s' is not found.", chart);
+        buffer_strcat(w->response.data, "Chart is not found: ");
+        buffer_strcat_htmlescape(w->response.data, chart);
         ret = 404;
         goto cleanup;
     }
@@ -1130,7 +1139,8 @@ int web_client_api_request_v1_data(struct web_client *w, char *url)
     RRDSET *st = rrdset_find(chart);
     if(!st) st = rrdset_find_byname(chart);
     if(!st) {
-        buffer_sprintf(w->response.data, "Chart '%s' is not found.", chart);
+        buffer_strcat(w->response.data, "Chart is not found: ");
+        buffer_strcat_htmlescape(w->response.data, chart);
         ret = 404;
         goto cleanup;
     }
@@ -1309,27 +1319,31 @@ int web_client_api_request_v1_registry(struct web_client *w, char *url)
     }
 
     if(action == 'A' && (!machine_guid || !machine_url || !url_name)) {
+        error("Invalid registry request - access requires these parameters: machine ('%s'), url ('%s'), name ('%s')",
+                machine_guid?machine_guid:"UNSET", machine_url?machine_url:"UNSET", url_name?url_name:"UNSET");
         buffer_flush(w->response.data);
-        buffer_sprintf(w->response.data, "Invalid registry request - access requires these parameters: machine ('%s'), url ('%s'), name ('%s')",
-                       machine_guid?machine_guid:"UNSET", machine_url?machine_url:"UNSET", url_name?url_name:"UNSET");
+        buffer_strcat(w->response.data, "Invalid registry Access request.");
         return 400;
     }
     else if(action == 'D' && (!machine_guid || !machine_url || !delete_url)) {
+        error("Invalid registry request - delete requires these parameters: machine ('%s'), url ('%s'), delete_url ('%s')",
+                machine_guid?machine_guid:"UNSET", machine_url?machine_url:"UNSET", delete_url?delete_url:"UNSET");
         buffer_flush(w->response.data);
-        buffer_sprintf(w->response.data, "Invalid registry request - delete requires these parameters: machine ('%s'), url ('%s'), delete_url ('%s')",
-                       machine_guid?machine_guid:"UNSET", machine_url?machine_url:"UNSET", delete_url?delete_url:"UNSET");
+        buffer_strcat(w->response.data, "Invalid registry Delete request.");
         return 400;
     }
     else if(action == 'S' && (!machine_guid || !machine_url || !search_machine_guid)) {
+        error("Invalid registry request - search requires these parameters: machine ('%s'), url ('%s'), for ('%s')",
+                machine_guid?machine_guid:"UNSET", machine_url?machine_url:"UNSET", search_machine_guid?search_machine_guid:"UNSET");
         buffer_flush(w->response.data);
-        buffer_sprintf(w->response.data, "Invalid registry request - search requires these parameters: machine ('%s'), url ('%s'), for ('%s')",
-                       machine_guid?machine_guid:"UNSET", machine_url?machine_url:"UNSET", search_machine_guid?search_machine_guid:"UNSET");
+        buffer_strcat(w->response.data, "Invalid registry Search request.");
         return 400;
     }
     else if(action == 'W' && (!machine_guid || !machine_url || !to_person_guid)) {
+        error("Invalid registry request - switching identity requires these parameters: machine ('%s'), url ('%s'), to ('%s')",
+                machine_guid?machine_guid:"UNSET", machine_url?machine_url:"UNSET", to_person_guid?to_person_guid:"UNSET");
         buffer_flush(w->response.data);
-        buffer_sprintf(w->response.data, "Invalid registry request - switching identity requires these parameters: machine ('%s'), url ('%s'), to ('%s')",
-                       machine_guid?machine_guid:"UNSET", machine_url?machine_url:"UNSET", to_person_guid?to_person_guid:"UNSET");
+        buffer_strcat(w->response.data, "Invalid registry Switch request.");
         return 400;
     }
 
@@ -1460,13 +1474,14 @@ int web_client_api_request_v1(struct web_client *w, char *url) {
 
         else {
             buffer_flush(w->response.data);
-            buffer_sprintf(w->response.data, "Unsupported v1 API command: %s", tok);
+            buffer_strcat(w->response.data, "Unsupported v1 API command: ");
+            buffer_strcat_htmlescape(w->response.data, tok);
             return 404;
         }
     }
     else {
         buffer_flush(w->response.data);
-        buffer_sprintf(w->response.data, "API v1 command?");
+        buffer_sprintf(w->response.data, "Which API v1 command?");
         return 400;
     }
 }
@@ -1481,7 +1496,8 @@ int web_client_api_request(struct web_client *w, char *url)
             return web_client_api_request_v1(w, url);
         else {
             buffer_flush(w->response.data);
-            buffer_sprintf(w->response.data, "Unsupported API version: %s", tok);
+            buffer_strcat(w->response.data, "Unsupported API version: ");
+            buffer_strcat_htmlescape(w->response.data, tok);
             return 404;
         }
     }
@@ -2098,14 +2114,16 @@ void web_client_process(struct web_client *w) {
                         if(!st) st = rrdset_find(tok);
                         if(!st) {
                             code = 404;
-                            buffer_sprintf(w->response.data, "Chart %s is not found.\r\n", tok);
+                            buffer_strcat(w->response.data, "Chart is not found: ");
+                            buffer_strcat_htmlescape(w->response.data, tok);
                             debug(D_WEB_CLIENT_ACCESS, "%llu: %s is not found.", w->id, tok);
                         }
                         else {
                             code = 200;
                             debug_flags |= D_RRD_STATS;
                             st->debug = !st->debug;
-                            buffer_sprintf(w->response.data, "Chart %s has now debug %s.\r\n", tok, st->debug?"enabled":"disabled");
+                            buffer_sprintf(w->response.data, "Chart has now debug %s: ", st->debug?"enabled":"disabled");
+                            buffer_strcat_htmlescape(w->response.data, tok);
                             debug(D_WEB_CLIENT_ACCESS, "%llu: debug for %s is %s.", w->id, tok, st->debug?"enabled":"disabled");
                         }
                     }