Hello!
On Tue, Jul 09, 2013 at 06:29:30PM -0700, Yichun Zhang (agentzh) wrote:
> Hello!
>
> I've found a heap buffer over-read issue in the Nginx core via clang's
> AddressSanitizer tool when Nginx is accepting a unix domain socket in
> ngx_event_accept.
>
> At least on Linux, accept and accept4 syscalls always return a socket
> length of 2 for unix domain sockets, which makes later accesses to
> saun->sun_path in function ngx_sock_ntop invalid (because
> sizeof(sa->sa_family) == sizeof(short) == 2).
Yep, there seems to be a problem with Linux accept() syscalls - it
returns invalid sockaddr.
> The patch attached fixes this issue.
>
> Thanks!
> -agentzh
>
> --- nginx-1.4.1/src/event/ngx_event_accept.c 2013-05-06 03:26:50.000000000
> -0700
> +++ nginx-1.4.1-patched/src/event/ngx_event_accept.c 2013-07-09
> 17:41:42.688468839 -0700
> @@ -268,7 +268,7 @@ ngx_event_accept(ngx_event_t *ev)
> wev->own_lock = &c->lock;
> #endif
>
> - if (ls->addr_ntop) {
> + if (ls->addr_ntop && socklen > sizeof(c->sockaddr->sa_family)) {
> c->addr_text.data = ngx_pnalloc(c->pool, ls->addr_text_max_len);
> if (c->addr_text.data == NULL) {
> ngx_close_accepted_connection(c);
The patch looks wrong - it doesn't initialize c->addr_text at all,
while it's requested by a caller.
--
Maxim Dounin
http://nginx.org/en/donation.html
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
