From a448dd52ee27ec3a550cb7d03fd27153f4799f0c Mon Sep 17 00:00:00 2001 From: Sergey Kandaurov Date: Thu, 21 Nov 2024 12:35:50 +0400 Subject: Upstream: disallow empty path in proxy_store and friends. Renaming a temporary file to an empty path ("") returns NGX_ENOPATH with a subsequent ngx_create_full_path() to create the full path. This function skips initial bytes as part of path separator lookup, which causes out of bounds access on short strings. The fix is to avoid renaming a temporary file to an obviously invalid path, as well as explicitly forbid such syntax for literal values. Although Coverity reports about potential type underflow, it is not actually possible because the terminating '\0' is always included. Notably, the run-time check is sufficient enough for Win32 as well. Other short invalid values result either in NGX_ENOENT or NGX_EEXIST and "MoveFile() .. failed" critical log messages, which involves a separate error handling. Prodded by Coverity (CID 1605485). --- src/http/ngx_http_upstream.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/http/ngx_http_upstream.c') diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c index 82a230024..d95662c56 100644 --- a/src/http/ngx_http_upstream.c +++ b/src/http/ngx_http_upstream.c @@ -4357,6 +4357,10 @@ ngx_http_upstream_store(ngx_http_request_t *r, ngx_http_upstream_t *u) "upstream stores \"%s\" to \"%s\"", tf->file.name.data, path.data); + if (path.len == 0) { + return; + } + (void) ngx_ext_rename_file(&tf->file.name, &path, &ext); u->store = 0; -- cgit