Opened 9 years ago

Closed 9 years ago

#141 closed defect (duplicate)

Nginx 1.1.18 Crash (off by one bug) -> Windows

Reported by: Benjamin Johnson Owned by: somebody
Priority: critical Milestone:
Component: nginx-core Version: 1.1.x
Keywords: Cc:
uname -a: windows 7 64-bit
nginx -V: 1.1.18

Description

Windows 7
Repeatable
Many Nginx versions, I reproduced it with Nginx 1.0.X (I believe several versions had this issue including 1.0.14, although I didn't do analysis), and from compiled source 1.1.18.

In src/os/win32/ngx_shmem.c, ngx_alloc does not account for the %Z (terminating \0). I was getting relatively frequent crashes on simple requests.

Index: src/os/win32/ngx_shmem.c
===================================================================
--- src/os/win32/ngx_shmem.c (revision 4568)
+++ src/os/win32/ngx_shmem.c (working copy)
@@ -15,7 +15,7 @@

u_char *name;
uint64_t size;


  • name = ngx_alloc(shm->name.len + 2 + sizeof(NGX_INT32_LEN), shm->log);

+ name = ngx_alloc(shm->name.len + 2 + sizeof(NGX_INT32_LEN) + 1, shm->log);

if (name == NULL) {

return NGX_ERROR;

}

Attachments (1)

off_by_one.diff (473 bytes ) - added by Benjamin Johnson 9 years ago.
patch

Download all attachments as: .zip

Change History (5)

by Benjamin Johnson, 9 years ago

Attachment: off_by_one.diff added

patch

comment:1 by Benjamin Johnson, 9 years ago

Changed to be more explicit for future developers:

Index: src/os/win32/ngx_shmem.c
===================================================================
--- src/os/win32/ngx_shmem.c (revision 4568)
+++ src/os/win32/ngx_shmem.c (working copy)
@@ -15,7 +15,7 @@

u_char *name;
uint64_t size;


  • name = ngx_alloc(shm->name.len + 2 + sizeof(NGX_INT32_LEN), shm->log);

+ name = ngx_alloc(shm->name.len + 2 + sizeof(NGX_INT32_LEN) + sizeof('\0'), shm->log);

if (name == NULL) {

return NGX_ERROR;

}

comment:2 by Vincent Lee, 9 years ago

what you found is as same as #134.but your fixing is not very good...

in reply to:  2 comment:3 by Benjamin Johnson, 9 years ago

Replying to 永平 李:

what you found is as same as #134.but your fixing is not very good...

Well your comment could be nicer, but thanks for pointing out a better fix.

comment:4 by Maxim Dounin, 9 years ago

Resolution: duplicate
Status: newclosed

Duplicate of #134.

Note: See TracTickets for help on using tickets.