[PATCH] Fixing buffer over-read when accepting unix domain sockets

[Maxim Dounin]

Hello!

On Wed, Jul 10, 2013 at 01:15:48PM -0700, Yichun Zhang (agentzh) wrote:

> Hello!
> 
> On Wed, Jul 10, 2013 at 6:18 AM, Maxim Dounin wrote:
> >> -        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.
> >
> 
> Thank you for the review!
> 
> How about this?

This doesn't looks good either.  It looks like on linux unix 
sockaddr can't be printed without socklen argument due to abstract 
namespace sockets (see [1]).  Therefore the only correct solution 
seems to be to change ngx_sock_ntop() interface to accept (and 
use) socklen.

Vladimir looked into this a while ago, and I've just reviewed his 
latest patch he resubmitted due to your attempts to fix the same 
issue.  The patch is good enough and expected to be committed 
after few minor fixes.

[1] http://man7.org/linux/man-pages/man7/unix.7.html
    Note "Three types of address are distinguished in this 
    structure..." and below.

-- 
Maxim Dounin
http://nginx.org/en/donation.html