netdev - Re: [iproute2 2/2] ss: fix NULL pointer access when parsing unix sockets with oldformat

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180106202818.2160a475@elisabeth>
Date:   Sat, 6 Jan 2018 20:28:18 +0100
From:   Stefano Brivio <sbrivio@...hat.com>
To:     Antonio Quartulli <a@...table.cc>
Cc:     netdev@...r.kernel.org,
        Stephen Hemminger <stephen@...workplumber.org>,
        Phil Sutter <phil@....cc>
Subject: Re: [iproute2 2/2] ss: fix NULL pointer access when parsing unix
 sockets with oldformat

On Sun,  7 Jan 2018 02:31:50 +0800
Antonio Quartulli <a@...table.cc> wrote:

> When parsing and printing the unix sockets in unix_show(),
> if the oldformat is detected, the peer_name member of the sockstat
> object is left uninitialized (NULL).

Luckily, it is initialized. I'd rather say:

	[...]
	object is not set (NULL).

> For this reason, if a filter has been specified on the command line,
> a strcmp() will crash when trying to access it.
> 
> Avoid crash by checking that peer_name is not NULL before
> passing it to strcmp().
> 
> Cc: Stefano Brivio <sbrivio@...hat.com>
> Cc: Stephen Hemminger <stephen@...workplumber.org>
> Signed-off-by: Antonio Quartulli <a@...table.cc>

Fixes: 2d0e538f3e1c ("ss: Drop list traversal from unix_stats_print()")

> [...]
>
> diff --git a/misc/ss.c b/misc/ss.c
> index b35859dc..29a25070 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -3711,7 +3711,10 @@ static int unix_show(struct filter *f)
>  			};
>  
>  			memcpy(st.local.data, &u->name, sizeof(u->name));
> -			if (strcmp(u->peer_name, "*"))
> +			/* when parsing the old format rport is set to 0 and
> +			 * therefore peer_name remains NULL
> +			 */

Maybe this comment is a bit redundant, but I don't have a strong
preference either.

> +			if (u->peer_name && strcmp(u->peer_name, "*"))
>  				memcpy(st.remote.data, &u->peer_name,
>  				       sizeof(u->peer_name));
>  			if (run_ssfilter(f->f, &st) == 0) {

FWIW:
Reviewed-by: Stefano Brivio <sbrivio@...hat.com>

-- 
Stefano