From 0d8b74930c74cc8018cb1c0cfc7cfe6740b49cf6 Mon Sep 17 00:00:00 2001 From: Udo Date: Thu, 21 May 2026 09:56:25 +0000 Subject: [PATCH] Harden HTTP path headers sessions and archives --- etc/uce/settings.cfg | 2 + site/tests/http.uce | 2 +- site/tests/security_headers.uce | 6 ++ src/fastcgi/src/fcgicc.cc | 110 +++++++++++++++++++++++++--- src/lib/sys.cpp | 23 ++++++ src/lib/sys.h | 2 + src/lib/uri.cpp | 67 ++++++++++++++++- src/lib/zip.cpp | 20 ++++- src/linux_fastcgi.cpp | 6 +- tests/plugins/uce_security_smoke.py | 44 +++++++++++ 10 files changed, 260 insertions(+), 22 deletions(-) create mode 100644 site/tests/security_headers.uce create mode 100644 tests/plugins/uce_security_smoke.py diff --git a/etc/uce/settings.cfg b/etc/uce/settings.cfg index da76ca0..70123c6 100644 --- a/etc/uce/settings.cfg +++ b/etc/uce/settings.cfg @@ -47,6 +47,8 @@ TRANSPORT_MAX_WEBSOCKET_OUTPUT_BYTES=4194304 TRANSPORT_MAX_RESPONSE_BYTES=8388608 TRANSPORT_HTTP_REQUEST_TIMEOUT_SECONDS=15 TRANSPORT_CONNECTION_IDLE_TIMEOUT_SECONDS=120 +# Empty means COMPILER_SYS_PATH for the built-in direct HTTP listener. +HTTP_DOCUMENT_ROOT= # CUSTOM SERVER LIMITS CUSTOM_SERVER_MAX_SERVERS=16 diff --git a/site/tests/http.uce b/site/tests/http.uce index 259e263..bc69437 100644 --- a/site/tests/http.uce +++ b/site/tests/http.uce @@ -40,7 +40,7 @@ RENDER(Request& context) check("uri_encode() / uri_decode()", decoded == uri_input, encoded + " => " + decoded); check("parse_query()", query["alpha"] == "1" && query["beta"] == "two words", query_dump); - check("set_cookie()", set_cookie_dump.find("site-tests-cookie") != String::npos && set_cookie_dump.find("cookie-value") != String::npos, set_cookie_dump); + check("set_cookie()", set_cookie_dump.find("site-tests-cookie") != String::npos && set_cookie_dump.find("cookie-value") != String::npos && set_cookie_dump.find("HttpOnly") != String::npos && set_cookie_dump.find("SameSite=Lax") != String::npos, set_cookie_dump); check("response header mutation", context.header["X-Site-Tests"] == "http-suite", header_dump); check("session_start()", session_id != "", "session_id=" + session_id); check("session_id_create()", generated_session.length() >= 16, generated_session); diff --git a/site/tests/security_headers.uce b/site/tests/security_headers.uce new file mode 100644 index 0000000..b2d40c7 --- /dev/null +++ b/site/tests/security_headers.uce @@ -0,0 +1,6 @@ +RENDER(Request& context) +{ + context.header["X-UCE-Security-Test"] = "safe\r\nX-UCE-Injected: nope"; + redirect("/tests/index.uce\r\nX-UCE-Redirect-Injected: nope", 299); + print("security header sanitizer test"); +} diff --git a/src/fastcgi/src/fcgicc.cc b/src/fastcgi/src/fcgicc.cc index 8930952..9cad8fc 100644 --- a/src/fastcgi/src/fcgicc.cc +++ b/src/fastcgi/src/fcgicc.cc @@ -36,6 +36,7 @@ #include #include +#include #include // E* #include @@ -123,6 +124,86 @@ make_http_text_response(String status_line, String body, String extra_headers = ); } +static bool +header_token_safe(String name) +{ + if(name == "") + return(false); + for(char c : name) + { + if(!(std::isalnum((unsigned char)c) || c == '-' || c == '_')) + return(false); + } + return(true); +} + +static String +header_value_sanitize(String value) +{ + for(char& c : value) + { + if(c == '\r' || c == '\n') + c = ' '; + } + return(value); +} + +static String +render_header_map(StringMap headers) +{ + String result; + for(auto& item : headers) + { + if(!header_token_safe(item.first)) + continue; + result += item.first + ": " + header_value_sanitize(item.second) + "\r\n"; + } + return(result); +} + +static String +render_set_cookie_headers(StringList headers) +{ + String result; + for(String header : headers) + { + if(header.find('\r') != String::npos || header.find('\n') != String::npos) + continue; + if(!str_starts_with(to_lower(header), "set-cookie: ")) + continue; + result += header + "\r\n"; + } + return(result); +} + +static String +safe_status_line(String status_line) +{ + if(status_line.find('\r') != String::npos || status_line.find('\n') != String::npos) + return("Status: 500 Internal Server Error"); + return(status_line); +} + +static String +http_script_root() +{ + if(context && context->server) + return(first( + context->server->config["HTTP_DOCUMENT_ROOT"], + context->server->config["COMPILER_SYS_PATH"], + cwd_get() + )); + return(cwd_get()); +} + +static String +strip_leading_slashes(String s) +{ + while(s.length() > 0 && s[0] == '/') + s.erase(0, 1); + return(s); +} + static bool is_valid_close_code(u16 status_code) { @@ -784,15 +865,20 @@ FastCGIServer::process_http_request(FastCGIRequest& request, String& data) if(!parse_http_message(request, data)) return; - if(request.params["HTTP_SCRIPT_FILENAME"] != "") - request.params["SCRIPT_FILENAME"] = request.params["HTTP_SCRIPT_FILENAME"]; - else if(request.params["SCRIPT_FILENAME"] == "" && request.params["DOCUMENT_URI"] != "") + if(request.params["SCRIPT_FILENAME"] == "" && request.params["DOCUMENT_URI"] != "") { - String document_root = first(request.params["DOCUMENT_ROOT"], cwd_get()); - if(document_root.length() > 1 && document_root[document_root.length()-1] == '/') - document_root.resize(document_root.length()-1); - request.params["DOCUMENT_ROOT"] = document_root; - request.params["SCRIPT_FILENAME"] = document_root + request.params["DOCUMENT_URI"]; + String document_root = http_script_root(); + String document_uri = strip_leading_slashes(request.params["DOCUMENT_URI"]); + String candidate = path_join(document_root, document_uri); + String real_root = path_real(document_root); + String real_candidate = path_real(candidate); + if(real_root == "" || real_candidate == "" || !path_is_within(real_candidate, real_root)) + { + reject_http_connection(*client_sockets[request.resources.client_socket], "HTTP/1.1 404 Not Found", "script not found\n"); + return; + } + request.params["DOCUMENT_ROOT"] = real_root; + request.params["SCRIPT_FILENAME"] = real_candidate; } if(to_lower(request.params["HTTP_UPGRADE"]) == "websocket") @@ -1253,9 +1339,9 @@ void FastCGIServer::assemble_output_buffer(FastCGIRequest& request, Connection* connection) { request.out = - request.response_code+"\r\n"+ - var_dump(request.header, "", "\r\n") + - var_dump(request.set_cookies, "", "\r\n") + + safe_status_line(request.response_code)+"\r\n"+ + render_header_map(request.header) + + render_set_cookie_headers(request.set_cookies) + "\r\n"; for(auto obs : request.ob_stack) @@ -1268,7 +1354,7 @@ FastCGIServer::assemble_output_buffer(FastCGIRequest& request, Connection* conne request.set_status(500, "Response Too Large"); request.header.clear(); request.header["Content-Type"] = "text/plain; charset=utf-8"; - request.out = request.response_code + "\r\n" + var_dump(request.header, "", "\r\n") + "\r\nresponse exceeded configured output limit\n"; + request.out = safe_status_line(request.response_code) + "\r\n" + render_header_map(request.header) + "\r\nresponse exceeded configured output limit\n"; } request.ob_stack.clear(); request.flags.output_closed = true; diff --git a/src/lib/sys.cpp b/src/lib/sys.cpp index a098453..6303fcf 100644 --- a/src/lib/sys.cpp +++ b/src/lib/sys.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include "sys.h" #include "hash.h" @@ -182,6 +183,27 @@ String path_join(String base, String child) return(base + "/" + child); } +String path_real(String path) +{ + char resolved[PATH_MAX]; + if(realpath(path.c_str(), resolved)) + return(String(resolved)); + return(""); +} + +bool path_is_within(String path, String root) +{ + String real_path = path_real(path); + String real_root = path_real(root); + if(real_path == "" || real_root == "") + return(false); + if(real_path == real_root) + return(true); + if(real_root[real_root.length() - 1] != '/') + real_root += "/"; + return(str_starts_with(real_path, real_root)); +} + bool mkdir(String path) { shell_exec(String("mkdir -p ")+" "+shell_escape(path)); @@ -877,6 +899,7 @@ StringMap make_server_settings() cfg["TRANSPORT_MAX_RESPONSE_BYTES"] = std::to_string(8 * 1024 * 1024); cfg["TRANSPORT_HTTP_REQUEST_TIMEOUT_SECONDS"] = "15"; cfg["TRANSPORT_CONNECTION_IDLE_TIMEOUT_SECONDS"] = "120"; + cfg["HTTP_DOCUMENT_ROOT"] = ""; cfg["CUSTOM_SERVER_MAX_SERVERS"] = "16"; cfg["CUSTOM_SERVER_MIN_PORT"] = "1024"; cfg["CUSTOM_SERVER_MAX_PORT"] = "65535"; diff --git a/src/lib/sys.h b/src/lib/sys.h index a61b182..d28be4b 100644 --- a/src/lib/sys.h +++ b/src/lib/sys.h @@ -9,6 +9,8 @@ String shell_escape(String raw); String basename(String fn); String dirname(String fn); String path_join(String base, String child); +String path_real(String path); +bool path_is_within(String path, String root); bool mkdir(String path); bool file_exists(String path); int file_open_locked(String file_name, int open_flags, int lock_type = LOCK_SH, int create_mode = 0644, f64 wait_timeout_seconds = 3.0, String purpose = ""); diff --git a/src/lib/uri.cpp b/src/lib/uri.cpp index bbbe2ec..1782e64 100644 --- a/src/lib/uri.cpp +++ b/src/lib/uri.cpp @@ -1,5 +1,8 @@ #include "uri.h" +#include +#include + static String base64_encode(String raw) { static const char* chars = @@ -256,9 +259,33 @@ String encode_query(StringMap map) return(result); } +namespace { + +String http_header_value_clean(String value) +{ + for(char& c : value) + { + if(c == '\r' || c == '\n') + c = ' '; + } + return(value); +} + +String cookie_attribute_value_clean(String value) +{ + for(char& c : value) + { + if(c == '\r' || c == '\n' || c == ';') + c = ' '; + } + return(trim(value)); +} + +} + void redirect(String url, s32 code) { - context->header["Location"] = url; + context->header["Location"] = http_header_value_clean(url); context->set_status(code); } @@ -499,9 +526,18 @@ void set_cookie( bool secure, bool http_only) { String cookie = "Set-Cookie: "; - cookie.append(uri_encode(name) + "=" + uri_encode(value)); + cookie.append(uri_encode(cookie_attribute_value_clean(name)) + "=" + uri_encode(value)); if(expires > 0) cookie.append(String("; Expires=") + time_format_utc("RFC1123", expires)); + if(path != "") + cookie.append("; Path=" + cookie_attribute_value_clean(path)); + if(domain != "") + cookie.append("; Domain=" + cookie_attribute_value_clean(domain)); + if(secure) + cookie.append("; Secure"); + if(http_only) + cookie.append("; HttpOnly"); + cookie.append("; SameSite=Lax"); context->set_cookies.push_back(cookie); context->cookies[name] = value; } @@ -520,7 +556,30 @@ StringMap parse_cookies(String cookie_String) String session_id_create() { - return(to_hex(rand())+to_hex(rand())+to_hex(rand())+to_hex(rand())); + unsigned char bytes[32]; + int fd = open("/dev/urandom", O_RDONLY | O_CLOEXEC); + if(fd == -1) + throw std::runtime_error("session_id_create(): could not open /dev/urandom"); + size_t offset = 0; + while(offset < sizeof(bytes)) + { + ssize_t count = read(fd, bytes + offset, sizeof(bytes) - offset); + if(count <= 0) + { + close(fd); + throw std::runtime_error("session_id_create(): could not read random bytes"); + } + offset += (size_t)count; + } + close(fd); + String result; + static const char* hex = "0123456789abcdef"; + for(unsigned char b : bytes) + { + result.push_back(hex[b >> 4]); + result.push_back(hex[b & 0x0f]); + } + return(result); } bool is_valid_session_id(String session_id) @@ -602,7 +661,7 @@ String session_start(String session_name) context->session_name = ""; String session_id = context->cookies[session_name]; - if(!is_valid_session_id(session_id)) + if(!is_valid_session_id(session_id) || !file_exists(session_file_path(session_id))) session_id = ""; if(session_id.length() == 0) diff --git a/src/lib/zip.cpp b/src/lib/zip.cpp index d29761f..75c3d77 100644 --- a/src/lib/zip.cpp +++ b/src/lib/zip.cpp @@ -122,6 +122,21 @@ String zip_read(String zip_file_name, String entry_name) if(!mz_zip_reader_init_file(&archive, zip_file_name.c_str(), 0)) throw std::runtime_error(zip_error("zip_read", "could not open " + zip_file_name)); + int file_index = mz_zip_reader_locate_file(&archive, entry_name.c_str(), NULL, 0); + if(file_index < 0) + { + mz_zip_reader_end(&archive); + throw std::runtime_error(zip_error("zip_read", "entry not found or not readable: " + entry_name)); + } + mz_zip_archive_file_stat stat; + std::memset(&stat, 0, sizeof(stat)); + if(!mz_zip_reader_file_stat(&archive, (mz_uint)file_index, &stat)) + { + mz_zip_reader_end(&archive); + throw std::runtime_error(zip_error("zip_read", "entry not found or not readable: " + entry_name)); + } + archive_check_size("zip_read", "uncompressed entry", stat.m_uncomp_size, "ARCHIVE_MAX_OUTPUT_BYTES", 64 * 1024 * 1024); + size_t size = 0; void* data = mz_zip_reader_extract_file_to_heap(&archive, entry_name.c_str(), &size, 0); if(!data) @@ -129,7 +144,6 @@ String zip_read(String zip_file_name, String entry_name) mz_zip_reader_end(&archive); throw std::runtime_error(zip_error("zip_read", "entry not found or not readable: " + entry_name)); } - archive_check_size("zip_read", "uncompressed entry", size, "ARCHIVE_MAX_OUTPUT_BYTES", 64 * 1024 * 1024); String result((char*)data, size); mz_free(data); mz_zip_reader_end(&archive); @@ -305,9 +319,12 @@ String gz_compress(String src) String gz_uncompress(String compressed) { + archive_check_size("gz_uncompress", "input", compressed.size(), "ARCHIVE_MAX_INPUT_BYTES", 64 * 1024 * 1024); size_t deflate_offset = gz_deflate_offset(compressed); size_t footer_offset = compressed.size() - 8; size_t deflate_size = footer_offset - deflate_offset; + u32 expected_size = gz_read_u32_le(compressed, footer_offset + 4); + archive_check_size("gz_uncompress", "declared output", expected_size, "ARCHIVE_MAX_OUTPUT_BYTES", 64 * 1024 * 1024); size_t out_len = 0; void* out = tinfl_decompress_mem_to_heap(compressed.data() + deflate_offset, deflate_size, &out_len, 0); if(!out) @@ -318,7 +335,6 @@ String gz_uncompress(String compressed) mz_free(out); u32 expected_crc = gz_read_u32_le(compressed, footer_offset); - u32 expected_size = gz_read_u32_le(compressed, footer_offset + 4); u32 actual_crc = (u32)mz_crc32(MZ_CRC32_INIT, (const unsigned char*)result.data(), result.size()); if(actual_crc != expected_crc) throw std::runtime_error("gz_uncompress(): crc check failed"); diff --git a/src/linux_fastcgi.cpp b/src/linux_fastcgi.cpp index 0f2b02c..8972d21 100644 --- a/src/linux_fastcgi.cpp +++ b/src/linux_fastcgi.cpp @@ -1329,10 +1329,10 @@ pid_t server_start_http(String key, String socket_fn_or_port, String call_uce_fi if(call_uce_filename[0] != '/') call_uce_filename = expand_path(call_uce_filename, cwd_get()); String allowed_root = first(server_state.config["CUSTOM_SERVER_UCE_ROOT"], path_join(server_state.config["COMPILER_SYS_PATH"], server_state.config["SITE_DIRECTORY"])); - if(allowed_root[allowed_root.length() - 1] != '/') - allowed_root += "/"; - if(!str_starts_with(call_uce_filename, allowed_root)) + String real_call_uce_filename = path_real(call_uce_filename); + if(real_call_uce_filename == "" || !path_is_within(real_call_uce_filename, allowed_root)) throw std::runtime_error("server_start_http(): call_uce_filename is outside configured custom server UCE root"); + call_uce_filename = real_call_uce_filename; String config_file = custom_server_config_file(key); StringMap previous_config; diff --git a/tests/plugins/uce_security_smoke.py b/tests/plugins/uce_security_smoke.py new file mode 100644 index 0000000..6d1e5bf --- /dev/null +++ b/tests/plugins/uce_security_smoke.py @@ -0,0 +1,44 @@ +import http.client + +from run_network_tests import TestFailure + + +def _direct_http_request(path, headers=None): + connection = http.client.HTTPConnection("127.0.0.1", 8080, timeout=5.0) + try: + connection.request("GET", path, headers=headers or {}) + response = connection.getresponse() + body = response.read().decode("utf-8", errors="replace") + return response.status, dict(response.getheaders()), body + finally: + connection.close() + + +def register(registry): + def direct_http_rejects_dotdot(context): + status, headers, body = _direct_http_request("/../site/demo/hello.uce") + if status == 200 or "hello world" in body: + raise TestFailure("direct HTTP accepted dot-dot script traversal") + return "direct HTTP dot-dot traversal rejected with HTTP %s" % status + + def direct_http_ignores_script_filename_header(context): + status, headers, body = _direct_http_request( + "/no-such-script.uce", + headers={"Script-Filename": "/Code/uce.openfu.com/uce/site/demo/hello.uce"}, + ) + if status == 200 or "hello world" in body: + raise TestFailure("direct HTTP trusted client Script-Filename header") + return "direct HTTP Script-Filename override rejected with HTTP %s" % status + + def response_headers_are_sanitized(context): + response = context.request("/tests/security_headers.uce") + if "X-UCE-Injected" in response.headers or "X-UCE-Redirect-Injected" in response.headers: + raise TestFailure("CRLF header injection produced an extra response header") + location = response.headers.get("Location", "") + if "\r" in location or "\n" in location: + raise TestFailure("Location header contains raw CR/LF") + return "CRLF response header injection was sanitized" + + registry.case("direct HTTP rejects dot-dot script traversal", direct_http_rejects_dotdot, tags=["security", "http", "internal"]) + registry.case("direct HTTP ignores Script-Filename header", direct_http_ignores_script_filename_header, tags=["security", "http", "internal"]) + registry.case("response headers sanitize CRLF", response_headers_are_sanitized, tags=["security", "http", "internal"])