Opened 2 years ago

Last modified 20 months ago

#2416 closed defect

Memory overrun due to alignment issues when cross-compiling mingw on linux — at Initial Version

Reported by: Orgad Shaneh Owned by:
Priority: minor Milestone:
Component: nginx-core Version: 1.22.x
Keywords: memory security vulnerability Cc: Orgad Shaneh
uname -a: MINGW64_NT-10.0-19043
nginx -V: nginx version: nginx/1.22.1
built with OpenSSL 1.1.1s 1 Nov 2022
TLS SNI support enabled
configure arguments: --crossbuild=win32 --with-http_stub_status_module --with-http_ssl_module --with-http_realip_module --without-http_scgi_module --without-http_uwsgi_module --with-openssl=../openssl --with-stream --with-stream_ssl_module --with-http_auth_request_module --with-http_sub_module --without-http_auth_basic_module --with-http_gzip_static_module --conf-path=nginx.conf --with-pcre=pcre --with-zlib=zlib-1.2.13

Description

Hi,

When I cross-compile nginx for Windows on linux host using mxe, NGX_ALIGNMENT is set to 16. This is set in auto/os/conf:

case "$NGX_MACHINE" in
...
    *)
        have=NGX_ALIGNMENT value=16 . auto/define
        NGX_MACH_CACHE_LINE=32
    ;;
esac

But the pool is allocated using plain malloc (on Windows target), which is *not* aligned to 16 bytes (it's aligned to 4), so pool->d.end is 4-byte aligned.

Now, in ngx_palloc_small, the following code causes integer underflow, and the comparison doesn't work as expected:

        m = p->d.last;

        if (align) {
            m = ngx_align_ptr(m, NGX_ALIGNMENT);
        }

        if ((size_t) (p->d.end - m) >= size) { /* <------- This condition */
            p->d.last = m + size;

            return m;
        }

Since m is aligned, and end is not, end - m can be a negative number. Then it is cast to size_t, which is unsigned, and so it just keeps writing past the memory pool, and overruns the next pool.

A minimal patch that solves this issue is:

  • src/core/ngx_palloc.c

    a b ngx_palloc_small(ngx_pool_t *pool, size_t size, ngx_uint_t align)  
    160160            m = ngx_align_ptr(m, NGX_ALIGNMENT);
    161161        }
    162162
    163         if ((size_t) (p->d.end - m) >= size) {
     163        if ((intptr_t) (p->d.end - m) >= (intptr_t)size) {
    164164            p->d.last = m + size;
    165165
    166166            return m;

But it would be better to assign the right alignment for Windows, even when cross-building. And also ensure that end is aligned correctly.

I also noticed that the default alignment when not set is sizeof(long), which is wrong for Win64. It should be sizeof(intptr_t).

Build command:

CC=i686-w64-mingw32.static-gcc ./auto/configure --crossbuild=win32 --without-http_rewrite_module --without-http_gzip_module

$ grep -1 ALIGN objs/ngx_auto_config.h

#ifndef NGX_ALIGNMENT
#define NGX_ALIGNMENT  16
#endif

Change History (0)

Note: See TracTickets for help on using tickets.