Script: buffer overrun protection.

With this change, all script copy operations now check if there is
enough room in the buffer.  To do so, the script engine now provides the
e->end pointer, which specifies expected buffer end, and each copy
operation is checked against it with the ngx_http_script_check_length()
function.

The e->end pointer is optional and only checked when set, thus
introducing no incompatible API changes.  All standard functions were
updated to use it, notably ngx_http_complex_value(), ngx_http_script_run(),
ngx_http_script_regex_start_code(), ngx_http_script_complex_value_code().
Direct script evaluation in the proxy, fastcgi, scgi, uwsgi, grpc proxy,
index, and try_files modules will be updated by a separate patch.

In particular, this catches issues as observed when evaluating variables
with side effects, such as in the following configuration:

    map $uri $map {
        ~(?<capture>.*) $capture;
    }

    set $capture "";
    set $temp "$capture $map";

As well as when evaluating non-cacheable variables, where length of a
variable might change between length and copy codes, such as in the
following configuration:

    map prefix:$capture $map_volatile {
        volatile;
        ~(?<capture>.*) $capture;
    }

    set $capture "";
    set $temp "$map_volatile";

Similar changes were made in the stream module.

Signed-off-by: Roman Arutyunyan <arut@nginx.com>
Origin: <https://freenginx.org/hg/nginx/rev/d172f506ab5f>

diff --git a/src/http/ngx_http_script.c b/src/http/ngx_http_script.c
index 8a7ba57e27ee1348d813a6a7cb426abdec140381..c7e28b23171199ae21cf6ad0999ea0f0491dcb64 100644 (file)
--- a/src/http/ngx_http_script.c
+++ b/src/http/ngx_http_script.c
@@ -92,12 +92,17 @@ ngx_http_complex_value(ngx_http_request_t *r, ngx_http_complex_value_t *val,
     e.ip = val->values;
     e.pos = value->data;
     e.buf = *value;
+    e.end = value->data + len;
 
     while (*(uintptr_t *) e.ip) {
         code = *(ngx_http_script_code_pt *) e.ip;
         code((ngx_http_script_engine_t *) &e);
     }
 
+    if (e.status) {
+        return NGX_ERROR;
+    }
+
     *value = e.buf;
 
     return NGX_OK;
@@ -652,12 +657,17 @@ ngx_http_script_run(ngx_http_request_t *r, ngx_str_t *value,
 
     e.ip = code_values;
     e.pos = value->data;
+    e.end = value->data + len;
 
     while (*(uintptr_t *) e.ip) {
         code = *(ngx_http_script_code_pt *) e.ip;
         code((ngx_http_script_engine_t *) &e);
     }
 
+    if (e.status) {
+        return NULL;
+    }
+
     return e.pos;
 }
 
@@ -807,6 +817,25 @@ ngx_http_script_add_code(ngx_array_t *codes, size_t size, void *code)
 }
 
 
+ngx_int_t
+ngx_http_script_check_length(ngx_http_script_engine_t *e, size_t len)
+{
+    if (e->end == NULL) {
+        return NGX_OK;
+    }
+
+    if (e->end - e->pos < (ssize_t) len) {
+        ngx_log_error(NGX_LOG_ALERT, e->request->connection->log, 0,
+                      "no buffer space in script copy");
+        e->ip = ngx_http_script_exit;
+        e->status = NGX_HTTP_INTERNAL_SERVER_ERROR;
+        return NGX_ERROR;
+    }
+
+    return NGX_OK;
+}
+
+
 static ngx_int_t
 ngx_http_script_add_copy_code(ngx_http_script_compile_t *sc, ngx_str_t *value,
     ngx_uint_t last)
@@ -875,6 +904,11 @@ ngx_http_script_copy_code(ngx_http_script_engine_t *e)
     p = e->pos;
 
     if (!e->skip) {
+
+        if (ngx_http_script_check_length(e, code->len) != NGX_OK) {
+            return;
+        }
+
         e->pos = ngx_copy(p, e->ip + sizeof(ngx_http_script_copy_code_t),
                           code->len);
     }
@@ -978,6 +1012,11 @@ ngx_http_script_copy_var_code(ngx_http_script_engine_t *e)
         }
 
         if (value && !value->not_found) {
+
+            if (ngx_http_script_check_length(e, value->len) != NGX_OK) {
+                return;
+            }
+
             p = e->pos;
             e->pos = ngx_copy(p, value->data, value->len);
 
@@ -1195,6 +1234,7 @@ ngx_http_script_regex_start_code(ngx_http_script_engine_t *e)
     e->quote = code->redirect;
 
     e->pos = e->buf.data;
+    e->end = e->buf.data + e->buf.len;
 
     e->ip += sizeof(ngx_http_script_regex_code_t);
 }
@@ -1232,6 +1272,11 @@ ngx_http_script_regex_end_code(ngx_http_script_engine_t *e)
         e->pos = dst;
 
         if (code->add_args && r->args.len) {
+
+            if (ngx_http_script_check_length(e, r->args.len + 1) != NGX_OK) {
+                return;
+            }
+
             *e->pos++ = (u_char) (code->args ? '&' : '?');
             e->pos = ngx_copy(e->pos, r->args.data, r->args.len);
         }
@@ -1265,6 +1310,11 @@ ngx_http_script_regex_end_code(ngx_http_script_engine_t *e)
         e->buf.len = e->args - e->buf.data;
 
         if (code->add_args && r->args.len) {
+
+            if (ngx_http_script_check_length(e, r->args.len + 1) != NGX_OK) {
+                return;
+            }
+
             *e->pos++ = '&';
             e->pos = ngx_copy(e->pos, r->args.data, r->args.len);
         }
@@ -1386,6 +1436,7 @@ ngx_http_script_copy_capture_code(ngx_http_script_engine_t *e)
     int                                  *cap;
     u_char                               *p, *pos;
     size_t                                len;
+    uintptr_t                             escape;
     ngx_uint_t                            n;
     ngx_http_request_t                   *r;
     ngx_http_script_copy_capture_code_t  *code;
@@ -1409,9 +1460,20 @@ ngx_http_script_copy_capture_code(ngx_http_script_engine_t *e)
         if ((e->is_args || e->quote)
             && (e->request->quoted_uri || e->request->plus_in_uri))
         {
+            escape = 2 * ngx_escape_uri(NULL, p, len, NGX_ESCAPE_ARGS);
+
+            if (ngx_http_script_check_length(e, len + escape) != NGX_OK) {
+                return;
+            }
+
             e->pos = (u_char *) ngx_escape_uri(pos, p, len, NGX_ESCAPE_ARGS);
 
         } else {
+
+            if (ngx_http_script_check_length(e, len) != NGX_OK) {
+                return;
+            }
+
             e->pos = ngx_copy(pos, p, len);
         }
     }
@@ -1796,6 +1858,7 @@ ngx_http_script_complex_value_code(ngx_http_script_engine_t *e)
     }
 
     e->pos = e->buf.data;
+    e->end = e->buf.data + len;
 
     e->sp->len = e->buf.len;
     e->sp->data = e->buf.data;

diff --git a/src/http/ngx_http_script.h b/src/http/ngx_http_script.h
index 43600383c1eb60af00fea9c772ed14f543036b5a..b086b266f38f67e398bfc6173be71a397ce61a80 100644 (file)
--- a/src/http/ngx_http_script.h
+++ b/src/http/ngx_http_script.h
@@ -17,6 +17,7 @@
 typedef struct {
     u_char                     *ip;
     u_char                     *pos;
+    u_char                     *end;
     ngx_http_variable_value_t  *sp;
 
     ngx_str_t                   buf;
@@ -240,6 +241,9 @@ void *ngx_http_script_start_code(ngx_pool_t *pool, ngx_array_t **codes,
     size_t size);
 void *ngx_http_script_add_code(ngx_array_t *codes, size_t size, void *code);
 
+ngx_int_t ngx_http_script_check_length(ngx_http_script_engine_t *e,
+    size_t len);
+
 size_t ngx_http_script_copy_len_code(ngx_http_script_engine_t *e);
 void ngx_http_script_copy_code(ngx_http_script_engine_t *e);
 size_t ngx_http_script_copy_var_len_code(ngx_http_script_engine_t *e);

diff --git a/src/stream/ngx_stream_script.c b/src/stream/ngx_stream_script.c
index d1f256caf1d4aca5493303ba1768b9cf6a5fbaf9..431364b49ef9ea67b8ea57067deb52a05833938c 100644 (file)
--- a/src/stream/ngx_stream_script.c
+++ b/src/stream/ngx_stream_script.c
@@ -92,6 +92,7 @@ ngx_stream_complex_value(ngx_stream_session_t *s,
 
     e.ip = val->values;
     e.pos = value->data;
+    e.end = value->data + len;
     e.buf = *value;
 
     while (*(uintptr_t *) e.ip) {
@@ -99,6 +100,10 @@ ngx_stream_complex_value(ngx_stream_session_t *s,
         code((ngx_stream_script_engine_t *) &e);
     }
 
+    if (e.status) {
+        return NGX_ERROR;
+    }
+
     *value = e.buf;
 
     return NGX_OK;
@@ -528,12 +533,17 @@ ngx_stream_script_run(ngx_stream_session_t *s, ngx_str_t *value,
 
     e.ip = code_values;
     e.pos = value->data;
+    e.end = value->data + len;
 
     while (*(uintptr_t *) e.ip) {
         code = *(ngx_stream_script_code_pt *) e.ip;
         code((ngx_stream_script_engine_t *) &e);
     }
 
+    if (e.status) {
+        return NULL;
+    }
+
     return e.pos;
 }
 
@@ -670,6 +680,25 @@ ngx_stream_script_add_code(ngx_array_t *codes, size_t size, void *code)
 }
 
 
+ngx_int_t
+ngx_stream_script_check_length(ngx_stream_script_engine_t *e, size_t len)
+{
+    if (e->end == NULL) {
+        return NGX_OK;
+    }
+
+    if (e->end - e->pos < (ssize_t) len) {
+        ngx_log_error(NGX_LOG_ALERT, e->session->connection->log, 0,
+                      "no buffer space in script copy");
+        e->ip = ngx_stream_script_exit;
+        e->status = NGX_STREAM_INTERNAL_SERVER_ERROR;
+        return NGX_ERROR;
+    }
+
+    return NGX_OK;
+}
+
+
 static ngx_int_t
 ngx_stream_script_add_copy_code(ngx_stream_script_compile_t *sc,
     ngx_str_t *value, ngx_uint_t last)
@@ -739,6 +768,11 @@ ngx_stream_script_copy_code(ngx_stream_script_engine_t *e)
     p = e->pos;
 
     if (!e->skip) {
+
+        if (ngx_stream_script_check_length(e, code->len) != NGX_OK) {
+            return;
+        }
+
         e->pos = ngx_copy(p, e->ip + sizeof(ngx_stream_script_copy_code_t),
                           code->len);
     }
@@ -843,6 +877,11 @@ ngx_stream_script_copy_var_code(ngx_stream_script_engine_t *e)
         }
 
         if (value && !value->not_found) {
+
+            if (ngx_stream_script_check_length(e, value->len) != NGX_OK) {
+                return;
+            }
+
             p = e->pos;
             e->pos = ngx_copy(p, value->data, value->len);
 
@@ -943,6 +982,11 @@ ngx_stream_script_copy_capture_code(ngx_stream_script_engine_t *e)
         cap = s->captures;
         len = cap[n + 1] - cap[n];
         p = s->captures_data + cap[n];
+
+        if (ngx_stream_script_check_length(e, len) != NGX_OK) {
+            return;
+        }
+
         e->pos = ngx_copy(pos, p, len);
     }
 
@@ -1015,6 +1059,7 @@ ngx_stream_script_full_name_code(ngx_stream_script_engine_t *e)
         != NGX_OK)
     {
         e->ip = ngx_stream_script_exit;
+        e->status = NGX_STREAM_INTERNAL_SERVER_ERROR;
         return;
     }
 

diff --git a/src/stream/ngx_stream_script.h b/src/stream/ngx_stream_script.h
index d8f3740470066ff6d28b8566a83884aa6a608931..dc6de9c72ca93eed72ed20db17d427a8ba72a672 100644 (file)
--- a/src/stream/ngx_stream_script.h
+++ b/src/stream/ngx_stream_script.h
@@ -17,6 +17,7 @@
 typedef struct {
     u_char                       *ip;
     u_char                       *pos;
+    u_char                       *end;
     ngx_stream_variable_value_t  *sp;
 
     ngx_str_t                     buf;
@@ -25,6 +26,7 @@ typedef struct {
     unsigned                      flushed:1;
     unsigned                      skip:1;
 
+    ngx_int_t                     status;
     ngx_stream_session_t         *session;
 } ngx_stream_script_engine_t;
 
@@ -127,6 +129,9 @@ void ngx_stream_script_flush_no_cacheable_variables(ngx_stream_session_t *s,
 
 void *ngx_stream_script_add_code(ngx_array_t *codes, size_t size, void *code);
 
+ngx_int_t ngx_stream_script_check_length(ngx_stream_script_engine_t *e,
+    size_t len);
+
 size_t ngx_stream_script_copy_len_code(ngx_stream_script_engine_t *e);
 void ngx_stream_script_copy_code(ngx_stream_script_engine_t *e);
 size_t ngx_stream_script_copy_var_len_code(ngx_stream_script_engine_t *e);