netdev - Re: [PATCH iproute2 1/5] ip: use strlcpy() to avoid truncation

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: <ac450d24-c966-2994-064e-4af83460ea1b@gmail.com>
Date:   Tue, 20 Mar 2018 09:44:12 -0600
From:   David Ahern <dsahern@...il.com>
To:     Stephen Hemminger <stephen@...workplumber.org>,
        netdev@...r.kernel.org
Subject: Re: [PATCH iproute2 1/5] ip: use strlcpy() to avoid truncation

On 3/19/18 10:56 AM, Stephen Hemminger wrote:
> diff --git a/lib/namespace.c b/lib/namespace.c
> index 6f3356d0fa08..682634028587 100644
> --- a/lib/namespace.c
> +++ b/lib/namespace.c
> @@ -23,7 +23,8 @@ static void bind_etc(const char *name)
>  	struct dirent *entry;
>  	DIR *dir;
>  
> -	snprintf(etc_netns_path, sizeof(etc_netns_path), "%s/%s", NETNS_ETC_DIR, name);
> +	snprintf(etc_netns_path, sizeof(etc_netns_path), "%s/%s",
> +		 NETNS_ETC_DIR, name);
>  	dir = opendir(etc_netns_path);
>  	if (!dir)
>  		return;
> @@ -33,7 +34,8 @@ static void bind_etc(const char *name)
>  			continue;
>  		if (strcmp(entry->d_name, "..") == 0)
>  			continue;
> -		snprintf(netns_name, sizeof(netns_name), "%s/%s", etc_netns_path, entry->d_name);
> +		snprintf(netns_name, sizeof(netns_name),
> +			 "%s/%s", etc_netns_path, entry->d_name);
>  		snprintf(etc_name, sizeof(etc_name), "/etc/%s", entry->d_name);
>  		if (mount(netns_name, etc_name, "none", MS_BIND, NULL) < 0) {
>  			fprintf(stderr, "Bind %s -> %s failed: %s\n",

above is unrelated to strncpy -> strlcpy change And pretty much
everything below as well.


> diff --git a/misc/nstat.c b/misc/nstat.c
> index a4dd405d43a9..433a1f483be3 100644
> --- a/misc/nstat.c
> +++ b/misc/nstat.c
> @@ -178,12 +178,12 @@ static int count_spaces(const char *line)
>  
>  static void load_ugly_table(FILE *fp)
>  {
> -	char buf[4096];
> +	char buf[2048];
>  	struct nstat_ent *db = NULL;
>  	struct nstat_ent *n;
>  
>  	while (fgets(buf, sizeof(buf), fp) != NULL) {
> -		char idbuf[sizeof(buf)];
> +		char idbuf[4096];

Also, I don't understand why you flipped the sizes with idbuf twice as
large as buf.


>  		int  off;
>  		char *p;
>  		int count1, count2, skip = 0;
> diff --git a/misc/ss.c b/misc/ss.c
> index e087bef739b0..a03fa4a7c174 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -4032,7 +4032,7 @@ static int netlink_show_one(struct filter *f,
>  
>  		if (!pid) {
>  			done = 1;
> -			strncpy(procname, "kernel", 6);
> +			strncpy(procname, "kernel", 7);


>  		} else if (pid > 0) {
>  			FILE *fp;
>  
> diff --git a/tc/m_ematch.c b/tc/m_ematch.c
> index d2bb5c380382..ee8981f0146c 100644
> --- a/tc/m_ematch.c
> +++ b/tc/m_ematch.c
> @@ -161,7 +161,7 @@ static struct ematch_util *get_ematch_kind(char *kind)
>  
>  static struct ematch_util *get_ematch_kind_num(__u16 kind)
>  {
> -	char name[32];
> +	char name[512];

why the size bump?

>  
>  	if (lookup_map(kind, name, sizeof(name), EMATCH_MAP) < 0)
>  		return NULL;
> diff --git a/tc/tc_class.c b/tc/tc_class.c
> index 1b214b82c702..91802518bb27 100644
> --- a/tc/tc_class.c
> +++ b/tc/tc_class.c
> @@ -219,7 +219,7 @@ static void graph_cls_show(FILE *fp, char *buf, struct hlist_head *root_list,
>  	char cls_id_str[256] = {};
>  	struct rtattr *tb[TCA_MAX + 1];
>  	struct qdisc_util *q;
> -	char str[100] = {};
> +	char str[300] = {};

and here.

>  
>  	hlist_for_each_safe(n, tmp_cls, root_list) {
>  		struct hlist_node *c, *tmp_chld;
> @@ -242,7 +242,8 @@ static void graph_cls_show(FILE *fp, char *buf, struct hlist_head *root_list,
>  		graph_indent(buf, cls, 0, 0);
>  
>  		print_tc_classid(cls_id_str, sizeof(cls_id_str), cls->id);
> -		sprintf(str, "+---(%s)", cls_id_str);
> +		snprintf(str, sizeof(str),
> +			 "+---(%s)", cls_id_str);

That can all fit on one line.

>  		strcat(buf, str);
>  
>  		parse_rtattr(tb, TCA_MAX, (struct rtattr *)cls->data,
>