Merge pull request NoiseByNorthwest#255 from NoiseByNorthwest/fix/251

Web UI: easier to reason about SPX_UI_URI confinement check

Author: NoiseByNorthwest

src/php_spx.c

@@ -924,24 +924,29 @@ static void http_ui_handler_shutdown(void)
         ui_uri = "/index.html";
     }
 
-    if (ui_uri[0] != '/' || strstr(ui_uri, "/../") != NULL) {
+    if (ui_uri[0] != '/') {
         goto error_404;
     }
 
     if (0 == http_ui_handler_data(SPX_G(data_dir), ui_uri)) {
         goto finish;
     }
 
-    char local_file_name[512];
-    snprintf(
-        local_file_name,
-        sizeof(local_file_name),
-        "%s%s",
-        SPX_G(http_ui_assets_dir),
-        ui_uri
-    );
+    char local_file_absolute_path[PATH_MAX];
+
+    if (
+        spx_utils_resolve_confined_file_absolute_path(
+            SPX_G(http_ui_assets_dir),
+            ui_uri,
+            NULL,
+            local_file_absolute_path,
+            sizeof(local_file_absolute_path)
+        ) == NULL
+    ) {
+        goto error_404;
+    }
 
-    if (0 == http_ui_handler_output_file(local_file_name)) {
+    if (0 == http_ui_handler_output_file(local_file_absolute_path)) {
         goto finish;
     }
 
@@ -1029,26 +1034,34 @@ static int http_ui_handler_data(const char * data_dir, const char *relative_path
 
     const char * get_report_metadata_uri = "/data/reports/metadata/";
     if (spx_utils_str_starts_with(relative_path, get_report_metadata_uri)) {
-        char file_name[512];
-        spx_reporter_full_build_metadata_file_name(
-            data_dir,
-            relative_path + strlen(get_report_metadata_uri),
-            file_name,
-            sizeof(file_name)
-        );
+        char file_name[PATH_MAX];
+        if (
+            spx_reporter_full_build_metadata_file_name(
+                data_dir,
+                relative_path + strlen(get_report_metadata_uri),
+                file_name,
+                sizeof(file_name)
+            ) == NULL
+        ) {
+            return -1;
+        }
 
         return http_ui_handler_output_file(file_name);
     }
 
     const char * get_report_uri = "/data/reports/get/";
     if (spx_utils_str_starts_with(relative_path, get_report_uri)) {
-        char file_name[512];
-        spx_reporter_full_build_file_name(
-            data_dir,
-            relative_path + strlen(get_report_uri),
-            file_name,
-            sizeof(file_name)
-        );
+        char file_name[PATH_MAX];
+        if (
+            spx_reporter_full_build_file_name(
+                data_dir,
+                relative_path + strlen(get_report_uri),
+                file_name,
+                sizeof(file_name)
+            ) == NULL
+        ) {
+            return -1;
+        }
 
         return http_ui_handler_output_file(file_name);
     }

src/spx_reporter_full.c

@@ -67,7 +67,7 @@ typedef struct {
 typedef struct {
     spx_profiler_reporter_t base;
 
-    char metadata_file_name[512];
+    char metadata_file_name[PATH_MAX];
     metadata_t * metadata;
     spx_output_stream_t * output;
 
@@ -99,7 +99,7 @@ size_t spx_reporter_full_metadata_list_files(
         return 0;
     }
 
-    char file_path[512];
+    char file_path[PATH_MAX];
     size_t count = 0;
     const struct dirent * entry;
     while ((entry = readdir(dir)) != NULL) {
@@ -124,33 +124,33 @@ size_t spx_reporter_full_metadata_list_files(
     return count;
 }
 
-int spx_reporter_full_build_metadata_file_name(
+char * spx_reporter_full_build_metadata_file_name(
     const char * data_dir,
     const char * key,
     char * file_name,
     size_t size
 ) {
-    return snprintf(
-        file_name,
-        size,
-        "%s/%s.json",
+    return spx_utils_resolve_confined_file_absolute_path(
         data_dir,
-        key
+        key,
+        ".json",
+        file_name,
+        size
     );
 }
 
-int spx_reporter_full_build_file_name(
+char * spx_reporter_full_build_file_name(
     const char * data_dir,
     const char * key,
     char * file_name,
     size_t size
 ) {
-    return snprintf(
-        file_name,
-        size,
-        "%s/%s.txt.gz",
+    return spx_utils_resolve_confined_file_absolute_path(
         data_dir,
-        key
+        key,
+        ".txt.gz",
+        file_name,
+        size
     );
 }
 
@@ -173,19 +173,21 @@ spx_profiler_reporter_t * spx_reporter_full_create(const char * data_dir)
         goto error;
     }
 
-    char file_name[512];
-    spx_reporter_full_build_file_name(
-        data_dir,
-        reporter->metadata->key,
+    char file_name[PATH_MAX];
+    snprintf(
         file_name,
-        sizeof(file_name)
+        sizeof(file_name),
+        "%s/%s.txt.gz",
+        data_dir,
+        reporter->metadata->key
     );
 
-    spx_reporter_full_build_metadata_file_name(
-        data_dir,
-        reporter->metadata->key,
+    snprintf(
         reporter->metadata_file_name,
-        sizeof(reporter->metadata_file_name)
+        sizeof(reporter->metadata_file_name),
+        "%s/%s.json",
+        data_dir,
+        reporter->metadata->key
     );
 
     (void) mkdir(data_dir, 0777);

src/spx_reporter_full.h

@@ -26,14 +26,14 @@ size_t spx_reporter_full_metadata_list_files(
     void (*callback) (const char *, size_t)
 );
 
-int spx_reporter_full_build_metadata_file_name(
+char * spx_reporter_full_build_metadata_file_name(
     const char * data_dir,
     const char * key,
     char * file_name,
     size_t size
 );
 
-int spx_reporter_full_build_file_name(
+char * spx_reporter_full_build_file_name(
     const char * data_dir,
     const char * key,
     char * file_name,

src/spx_utils.c

@@ -21,6 +21,52 @@
 #include <string.h>
 #include "spx_utils.h"
 
+char * spx_utils_resolve_confined_file_absolute_path(
+    const char * root_dir,
+    const char * relative_path,
+    const char * suffix,
+    char * dst,
+    size_t size
+) {
+    if (size < PATH_MAX) {
+        spx_utils_die("size < PATH_MAX");
+    }
+
+    char absolute_file_path[PATH_MAX];
+
+    snprintf(
+        absolute_file_path,
+        sizeof(absolute_file_path),
+        "%s%s%s",
+        root_dir,
+        relative_path,
+        suffix == NULL ? "" : suffix
+    );
+
+    if (realpath(absolute_file_path, dst) == NULL) {
+        return NULL;
+    }
+
+    char root_dir_real_path[PATH_MAX];
+    if (realpath(root_dir, root_dir_real_path) == NULL) {
+        return NULL;
+    }
+
+    char expected_path_prefix[PATH_MAX + 1];
+    snprintf(
+        expected_path_prefix,
+        sizeof(expected_path_prefix),
+        "%s/",
+        root_dir_real_path
+    );
+
+    if (! spx_utils_str_starts_with(dst, expected_path_prefix)) {
+        return NULL;
+    }
+
+    return dst;
+}
+
 char * spx_utils_json_escape(char * dst, const char * src, size_t limit)
 {
     size_t i = 0;

src/spx_utils.h

@@ -20,6 +20,7 @@
 #define SPX_UTILS_H_DEFINED
 
 #include <stddef.h>
+#include <limits.h> /* PATH_MAX */
 
 #define SPX_UTILS_TOKENIZE_STRING(str, delim, token, size, block) \
 do {                                                              \
@@ -49,6 +50,14 @@ do {                                                              \
     }                                                             \
 } while (0)
 
+char * spx_utils_resolve_confined_file_absolute_path(
+    const char * root_dir,
+    const char * relative_path,
+    const char * suffix,
+    char * dst,
+    size_t size
+);
+
 char * spx_utils_json_escape(char * dst, const char * src, size_t limit);
 int spx_utils_str_starts_with(const char * str, const char * prefix);
 int spx_utils_str_ends_with(const char * str, const char * suffix);

tests/spx_auth_ip_ko.phpt

@@ -5,6 +5,7 @@ Authentication: KO (invalid IP address)
 spx.http_enabled=1
 spx.http_key="dev"
 spx.http_ip_whitelist="127.0.0.1,127.0.0.2,127.0.0.3"
+spx.http_ui_assets_dir="{PWD}/../assets/web-ui"
 log_errors=on
 --ENV--
 return <<<END

tests/spx_auth_ip_ok.phpt

@@ -5,6 +5,7 @@ Authentication: OK (valid IP address)
 spx.http_enabled=1
 spx.http_key="dev"
 spx.http_ip_whitelist="127.0.0.1,127.0.0.2,127.0.0.3"
+spx.http_ui_assets_dir="{PWD}/../assets/web-ui"
 log_errors=on
 --ENV--
 return <<<END

tests/spx_auth_ip_trusted_proxy_ko.phpt

@@ -7,6 +7,7 @@ spx.http_key="dev"
 spx.http_ip_var="HTTP_X_FORWARDED_FOR"
 spx.http_trusted_proxies="127.0.0.2,127.0.0.3,127.0.0.4"
 spx.http_ip_whitelist="127.0.0.5"
+spx.http_ui_assets_dir="{PWD}/../assets/web-ui"
 log_errors=on
 --ENV--
 return <<<END

tests/spx_auth_ip_trusted_proxy_ok.phpt

@@ -7,6 +7,7 @@ spx.http_key="dev"
 spx.http_ip_var="HTTP_X_FORWARDED_FOR"
 spx.http_trusted_proxies="127.0.0.2,127.0.0.3,127.0.0.4"
 spx.http_ip_whitelist="127.0.0.5"
+spx.http_ui_assets_dir="{PWD}/../assets/web-ui"
 log_errors=on
 --ENV--
 return <<<END

tests/spx_auth_ipv6_ko.phpt

@@ -5,6 +5,7 @@ Authentication: KO (invalid IPv6 address)
 spx.http_enabled=1
 spx.http_key="dev"
 spx.http_ip_whitelist="0000:0000:0000:0000:0000:ffff:127.0.0.1,0000:0000:0000:0000:0000:ffff:127.0.0.2,0000:0000:0000:0000:0000:ffff:127.0.0.3"
+spx.http_ui_assets_dir="{PWD}/../assets/web-ui"
 log_errors=on
 --ENV--
 return <<<END