Fetch: fixed heap buffer overflow in proxy URL credentials.

The destination buffers for the decoded user and password in
ngx_js_parse_proxy_url() were a fixed 128 bytes, while the encoded
input length was bounded only by the URL length.  Since
ngx_unescape_uri() writes at most one byte per input byte, raw
credentials longer than 128 bytes overflowed the buffer; the
length check ran only after the decode.

The fix is to size the destination buffer based on the encoded
input length.

The bug appeared in dea8318 (0.9.4).

diff --git a/nginx/ngx_js.c b/nginx/ngx_js.c
index da50d766a6d7f3fcbc8d3147e8b1787f454a7401..f3ca74d8501b46daee392e66e21dd3fbbf4848eb 100644 (file)
--- a/nginx/ngx_js.c
+++ b/nginx/ngx_js.c
@@ -3549,12 +3549,12 @@ ngx_js_parse_proxy_url(ngx_pool_t *pool, ngx_log_t *log, ngx_str_t *url,
         pass_start = colon + 1;
         pass_len = at - pass_start;
 
-        decoded_user = ngx_pnalloc(pool, 128);
+        decoded_user = ngx_pnalloc(pool, user_len);
         if (decoded_user == NULL) {
             return NGX_ERROR;
         }
 
-        decoded_pass = ngx_pnalloc(pool, 128);
+        decoded_pass = ngx_pnalloc(pool, pass_len);
         if (decoded_pass == NULL) {
             return NGX_ERROR;
         }
@@ -3562,24 +3562,22 @@ ngx_js_parse_proxy_url(ngx_pool_t *pool, ngx_log_t *log, ngx_str_t *url,
         p = user_start;
         decoded_end = decoded_user;
         ngx_unescape_uri(&decoded_end, &p, user_len, NGX_UNESCAPE_URI);
-
         user_len = decoded_end - decoded_user;
-        if (user_len == 0 || user_len > 127) {
+
+        if (user_len == 0) {
             ngx_log_error(NGX_LOG_ERR, log, 0,
-                          "js_fetch_proxy username invalid or too long "
-                          "(max 127 bytes after decoding)");
+                          "js_fetch_proxy username is empty");
             return NGX_ERROR;
         }
 
         p = pass_start;
         decoded_end = decoded_pass;
         ngx_unescape_uri(&decoded_end, &p, pass_len, NGX_UNESCAPE_URI);
-
         pass_len = decoded_end - decoded_pass;
-        if (pass_len == 0 || pass_len > 127) {
+
+        if (pass_len == 0) {
             ngx_log_error(NGX_LOG_ERR, log, 0,
-                          "js_fetch_proxy password invalid or too long "
-                          "(max 127 bytes after decoding)");
+                          "js_fetch_proxy password is empty");
             return NGX_ERROR;
         }
 

diff --git a/nginx/t/js_fetch_proxy_variable.t b/nginx/t/js_fetch_proxy_variable.t
index 7399d28f6177cb917d8adfb7494fd0d49e8dab98..b1fcbadda8c8909ba3ea8b787d574b8687727eb3 100644 (file)
--- a/nginx/t/js_fetch_proxy_variable.t
+++ b/nginx/t/js_fetch_proxy_variable.t
@@ -59,6 +59,12 @@ http {
             js_fetch_proxy $proxy_url;
             js_content test.http_fetch;
         }
+
+        location /dynamic_user_proxy {
+            set $proxy_url "http://$arg_user:p@127.0.0.1:%%PORT_8081%%";
+            js_fetch_proxy $proxy_url;
+            js_content test.http_fetch;
+        }
     }
 
     server {
@@ -128,7 +134,7 @@ $t->write_file('test.js', <<EOF);
 
 EOF
 
-$t->try_run('no js_fetch_proxy')->plan(3);
+$t->try_run('no js_fetch_proxy')->plan(4);
 
 ###############################################################################
 
@@ -138,5 +144,8 @@ like(http_get('/dynamic_proxy'), qr/PROXY:Basic\s+dGVzdHVzZXI6dGVzdHBhc3M=/,
     'dynamic proxy URL with auth');
 like(http_get('/dynamic_empty_proxy'), qr/ORIGIN:OK/,
     'dynamic empty proxy URL bypasses proxy');
+like(http_get('/dynamic_user_proxy?user=' . ('a' x 200)),
+    qr/PROXY:BAD-AUTH/,
+    'long user in dynamic proxy URL decoded without overflow');
 
 ###############################################################################