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).

diff --git a/src/http/modules/ngx_http_fastcgi_module.c b/src/http/modules/ngx_http_fastcgi_module.c
index 743fe0c0c701b8e371b9f3106a0335bf5c109adc..a41b496dc8f6c0c7b704114007ab01014ca97c44 100644 (file)
--- a/src/http/modules/ngx_http_fastcgi_module.c
+++ b/src/http/modules/ngx_http_fastcgi_module.c
@@ -3781,6 +3781,11 @@ ngx_http_fastcgi_store(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
         return NGX_CONF_OK;
     }
 
+    if (value[1].len == 0) {
+        ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "empty path");
+        return NGX_CONF_ERROR;
+    }
+
 #if (NGX_HTTP_CACHE)
     if (flcf->upstream.cache > 0) {
         return "is incompatible with \"fastcgi_cache\"";

diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c
index 25fa92bae32f691b0b0fc14d6ff37545fa7bba93..855ef523dd81c3da97f09af18b2e51ab6706c1fb 100644 (file)
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -4943,6 +4943,11 @@ ngx_http_proxy_store(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
         return NGX_CONF_OK;
     }
 
+    if (value[1].len == 0) {
+        ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "empty path");
+        return NGX_CONF_ERROR;
+    }
+
 #if (NGX_HTTP_CACHE)
     if (plcf->upstream.cache > 0) {
         return "is incompatible with \"proxy_cache\"";

diff --git a/src/http/modules/ngx_http_scgi_module.c b/src/http/modules/ngx_http_scgi_module.c
index 671cd2cb7356fc2290ba5efccd4ccc70e118f7d5..9023a36e49c54396f959f83956d2641a6c5f030d 100644 (file)
--- a/src/http/modules/ngx_http_scgi_module.c
+++ b/src/http/modules/ngx_http_scgi_module.c
@@ -1995,6 +1995,11 @@ ngx_http_scgi_store(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
         return NGX_CONF_OK;
     }
 
+    if (value[1].len == 0) {
+        ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "empty path");
+        return NGX_CONF_ERROR;
+    }
+
 #if (NGX_HTTP_CACHE)
     if (scf->upstream.cache > 0) {
         return "is incompatible with \"scgi_cache\"";

diff --git a/src/http/modules/ngx_http_uwsgi_module.c b/src/http/modules/ngx_http_uwsgi_module.c
index f42ae706ae35a5befa6590d9b16e93f96d4b60fc..7988cc589e2b61304cce1decc8b47ac4f8eea00a 100644 (file)
--- a/src/http/modules/ngx_http_uwsgi_module.c
+++ b/src/http/modules/ngx_http_uwsgi_module.c
@@ -2322,6 +2322,11 @@ ngx_http_uwsgi_store(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
         return NGX_CONF_OK;
     }
 
+    if (value[1].len == 0) {
+        ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "empty path");
+        return NGX_CONF_ERROR;
+    }
+
 #if (NGX_HTTP_CACHE)
 
     if (uwcf->upstream.cache > 0) {

diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
index 82a2300248c609ccca089398c6baa8e0dfc27def..d95662c56612f0a2b8d67a2726211501423f3f98 100644 (file)
--- 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;