[PATCH] Fixing memory overflow issues in ngx_resolver's debug logging code

[Ruslan Ermilov]

On Fri, Jun 01, 2012 at 06:14:58PM +0800, agentzh wrote:
> I've noticed a small memory overflow issue in ngx_resolver's debug
> logging code that was caught by Valgrind/Memcheck on Linux x86_64.

Calling it "memory overflow" is misleading -- nginx doesn't write
beyond some buffer of memory.  Instead, nginx will try to access
(read) more bytes of memory than expected (e.g., 8 instead of 4),
and in theory it can access unallocated or uninitialized memory.
In practice, there's no problem on PC because memory is allocated in
pages, allocations on stack are aligned, and stack memory pages are
pre-zeroed when allocated.

> Basically, when calling ngx_log_debug6 from within
> ngx_resolver_process_response, the "%ui" formatter is incorrectly used
> for int-typed values "(query->nns_hi << 8) + query->nns_lo" and
> "(query->nar_hi << 8) + query->nar_lo".
> 
> Below attaches a patch for nginx 1.3.0 :)
> 
> Hope this helps,
> -agentzh
> 
> --- nginx-1.3.0/src/core/ngx_resolver.c	2012-05-14 17:13:45.000000000 +0800
> +++ nginx-1.3.0-patched/src/core/ngx_resolver.c	2012-06-01
> 18:08:06.512047421 +0800
> @@ -1035,7 +1035,7 @@
>      nan = (query->nan_hi << 8) + query->nan_lo;
> 
>      ngx_log_debug6(NGX_LOG_DEBUG_CORE, r->log, 0,
> -                   "resolver DNS response %ui fl:%04Xui %ui/%ui/%ui/%ui",
> +                   "resolver DNS response %ui fl:%04Xui %ui/%ui/%ud/%ud",
>                     ident, flags, nqs, nan,
>                     (query->nns_hi << 8) + query->nns_lo,
>                     (query->nar_hi << 8) + query->nar_lo);

I think a better approach would be to cast the last two expressions
to ngx_uint_t, like is done for other expressions (via assignments):

%%%
Index: src/core/ngx_resolver.c
===================================================================
--- src/core/ngx_resolver.c	(revision 4653)
+++ src/core/ngx_resolver.c	(working copy)
@@ -1042,8 +1042,8 @@ ngx_resolver_process_response(ngx_resolv
     ngx_log_debug6(NGX_LOG_DEBUG_CORE, r->log, 0,
                    "resolver DNS response %ui fl:%04Xui %ui/%ui/%ui/%ui",
                    ident, flags, nqs, nan,
-                   (query->nns_hi << 8) + query->nns_lo,
-                   (query->nar_hi << 8) + query->nar_lo);
+                   (ngx_uint_t) ((query->nns_hi << 8) + query->nns_lo),
+                   (ngx_uint_t) ((query->nar_hi << 8) + query->nar_lo));
 
     if (!(flags & 0x8000)) {
         ngx_log_error(r->log_level, r->log, 0,
%%%