Geo: check binary base CRC32 before relocation

This provides safe pointer handling in the event of a corrupted file.

diff --git a/src/http/modules/ngx_http_geo_module.c b/src/http/modules/ngx_http_geo_module.c
index 5018e1abb572b7e636c42b368dea1b491357a49f..4c8e7c6abca49eb9f15faa2139b2ae9a66bd59d0 100644 (file)
--- a/src/http/modules/ngx_http_geo_module.c
+++ b/src/http/modules/ngx_http_geo_module.c
@@ -1423,7 +1423,6 @@ ngx_http_geo_include_binary_base(ngx_conf_t *cf, ngx_http_geo_conf_ctx_t *ctx,
     time_t                      mtime;
     size_t                      size, len;
     ssize_t                     n;
-    uint32_t                    crc32;
     ngx_err_t                   err;
     ngx_int_t                   rc;
     ngx_uint_t                  i;
@@ -1518,24 +1517,45 @@ ngx_http_geo_include_binary_base(ngx_conf_t *cf, ngx_http_geo_conf_ctx_t *ctx,
         goto failed;
     }
 
-    ngx_crc32_init(crc32);
+    /*
+     * the base is walked below without bounds checking, so make sure
+     * it is intact first: the checksum in the header is computed over
+     * the entire body, which the walk covers contiguously
+     */
+
+    if (size < sizeof(ngx_http_geo_header_t)
+               + sizeof(ngx_http_variable_value_t)
+               + 0x10000 * sizeof(ngx_http_geo_range_t *))
+    {
+        ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+                           "truncated binary geo range base \"%s\"",
+                           name->data);
+        goto failed;
+    }
+
+    if (ngx_crc32_long(base + sizeof(ngx_http_geo_header_t),
+                       size - sizeof(ngx_http_geo_header_t))
+        != header->crc32)
+    {
+        ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+                           "CRC32 mismatch in binary geo range base \"%s\"",
+                           name->data);
+        goto failed;
+    }
 
     vv = (ngx_http_variable_value_t *) (base + sizeof(ngx_http_geo_header_t));
 
     while (vv->data) {
         len = ngx_align(sizeof(ngx_http_variable_value_t) + vv->len,
                         sizeof(void *));
-        ngx_crc32_update(&crc32, (u_char *) vv, len);
         vv->data += (size_t) base;
         vv = (ngx_http_variable_value_t *) ((u_char *) vv + len);
     }
-    ngx_crc32_update(&crc32, (u_char *) vv, sizeof(ngx_http_variable_value_t));
     vv++;
 
     ranges = (ngx_http_geo_range_t **) vv;
 
     for (i = 0; i < 0x10000; i++) {
-        ngx_crc32_update(&crc32, (u_char *) &ranges[i], sizeof(void *));
         if (ranges[i]) {
             ranges[i] = (ngx_http_geo_range_t *)
                             ((u_char *) ranges[i] + (size_t) base);
@@ -1546,24 +1566,13 @@ ngx_http_geo_include_binary_base(ngx_conf_t *cf, ngx_http_geo_conf_ctx_t *ctx,
 
     while ((u_char *) range < base + size) {
         while (range->value) {
-            ngx_crc32_update(&crc32, (u_char *) range,
-                             sizeof(ngx_http_geo_range_t));
             range->value = (ngx_http_variable_value_t *)
                                ((u_char *) range->value + (size_t) base);
             range++;
         }
-        ngx_crc32_update(&crc32, (u_char *) range, sizeof(void *));
         range = (ngx_http_geo_range_t *) ((u_char *) range + sizeof(void *));
     }
 
-    ngx_crc32_final(crc32);
-
-    if (crc32 != header->crc32) {
-        ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
-                  "CRC32 mismatch in binary geo range base \"%s\"", name->data);
-        goto failed;
-    }
-
     ngx_conf_log_error(NGX_LOG_NOTICE, cf, 0,
                        "using binary geo range base \"%s\"", name->data);
 

diff --git a/src/stream/ngx_stream_geo_module.c b/src/stream/ngx_stream_geo_module.c
index 316de92869d82c3a831548f2de923b34a46569ee..88d9d723cb25bbee772f6a1ce7c08baca66f6edb 100644 (file)
--- a/src/stream/ngx_stream_geo_module.c
+++ b/src/stream/ngx_stream_geo_module.c
@@ -1349,7 +1349,6 @@ ngx_stream_geo_include_binary_base(ngx_conf_t *cf,
     time_t                        mtime;
     size_t                        size, len;
     ssize_t                       n;
-    uint32_t                      crc32;
     ngx_err_t                     err;
     ngx_int_t                     rc;
     ngx_uint_t                    i;
@@ -1444,7 +1443,31 @@ ngx_stream_geo_include_binary_base(ngx_conf_t *cf,
         goto failed;
     }
 
-    ngx_crc32_init(crc32);
+    /*
+     * the base is walked below without bounds checking, so make sure
+     * it is intact first: the checksum in the header is computed over
+     * the entire body, which the walk covers contiguously
+     */
+
+    if (size < sizeof(ngx_stream_geo_header_t)
+               + sizeof(ngx_stream_variable_value_t)
+               + 0x10000 * sizeof(ngx_stream_geo_range_t *))
+    {
+        ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+                           "truncated binary geo range base \"%s\"",
+                           name->data);
+        goto failed;
+    }
+
+    if (ngx_crc32_long(base + sizeof(ngx_stream_geo_header_t),
+                       size - sizeof(ngx_stream_geo_header_t))
+        != header->crc32)
+    {
+        ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+                           "CRC32 mismatch in binary geo range base \"%s\"",
+                           name->data);
+        goto failed;
+    }
 
     vv = (ngx_stream_variable_value_t *)
             (base + sizeof(ngx_stream_geo_header_t));
@@ -1452,18 +1475,14 @@ ngx_stream_geo_include_binary_base(ngx_conf_t *cf,
     while (vv->data) {
         len = ngx_align(sizeof(ngx_stream_variable_value_t) + vv->len,
                         sizeof(void *));
-        ngx_crc32_update(&crc32, (u_char *) vv, len);
         vv->data += (size_t) base;
         vv = (ngx_stream_variable_value_t *) ((u_char *) vv + len);
     }
-    ngx_crc32_update(&crc32, (u_char *) vv,
-                     sizeof(ngx_stream_variable_value_t));
     vv++;
 
     ranges = (ngx_stream_geo_range_t **) vv;
 
     for (i = 0; i < 0x10000; i++) {
-        ngx_crc32_update(&crc32, (u_char *) &ranges[i], sizeof(void *));
         if (ranges[i]) {
             ranges[i] = (ngx_stream_geo_range_t *)
                             ((u_char *) ranges[i] + (size_t) base);
@@ -1474,24 +1493,13 @@ ngx_stream_geo_include_binary_base(ngx_conf_t *cf,
 
     while ((u_char *) range < base + size) {
         while (range->value) {
-            ngx_crc32_update(&crc32, (u_char *) range,
-                             sizeof(ngx_stream_geo_range_t));
             range->value = (ngx_stream_variable_value_t *)
                                ((u_char *) range->value + (size_t) base);
             range++;
         }
-        ngx_crc32_update(&crc32, (u_char *) range, sizeof(void *));
         range = (ngx_stream_geo_range_t *) ((u_char *) range + sizeof(void *));
     }
 
-    ngx_crc32_final(crc32);
-
-    if (crc32 != header->crc32) {
-        ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
-                  "CRC32 mismatch in binary geo range base \"%s\"", name->data);
-        goto failed;
-    }
-
     ngx_conf_log_error(NGX_LOG_NOTICE, cf, 0,
                        "using binary geo range base \"%s\"", name->data);