linux-kernel - Re: [PATCH v2] hv/hv_kvp_daemon: Handle IPv4 and Ipv6 combination for keyfile format

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] [day] [month] [year] [list]

Message-ID: <20240318020255.GA28704@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Date: Sun, 17 Mar 2024 19:02:55 -0700
From: Shradha Gupta <shradhagupta@...ux.microsoft.com>
To: Easwar Hariharan <eahariha@...ux.microsoft.com>
Cc: linux-kernel@...r.kernel.org, linux-hyperv@...r.kernel.org,
	"K. Y. Srinivasan" <kys@...rosoft.com>,
	Haiyang Zhang <haiyangz@...rosoft.com>,
	Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
	Long Li <longli@...rosoft.com>,
	Michael Kelley <mikelley@...rosoft.com>,
	Olaf Hering <olaf@...fle.de>, Ani Sinha <anisinha@...hat.com>,
	Shradha Gupta <shradhagupta@...rosoft.com>
Subject: Re: [PATCH v2] hv/hv_kvp_daemon: Handle IPv4 and Ipv6 combination
 for keyfile format

On Fri, Mar 15, 2024 at 09:56:22AM -0700, Easwar Hariharan wrote:
> On 3/15/2024 7:09 AM, Shradha Gupta wrote:
> 
> <snip>
> 
> >>>>  }
> >>>>  
> >>>> +static int process_dns_gateway_nm(FILE *f, char *ip_string, int type,
> >>>> +				  int ip_sec)
> >>>> +{
> >>>> +	char addr[INET6_ADDRSTRLEN], *output_str;
> >>>> +	int ip_offset = 0, error = 0, ip_ver;
> >>>> +	char *param_name;
> >>>> +
> >>>> +	output_str = (char *)calloc(INET6_ADDRSTRLEN * MAX_IP_ENTRIES,
> >>>> +				    sizeof(char));
> >>>> +
> >>>> +	if (!output_str)
> >>>> +		return -ENOMEM;
> >>>> +
> >>>> +	memset(addr, 0, sizeof(addr));
> >>>> +
> >>>> +	if (type == DNS) {
> >>>> +		param_name = "dns";
> >>>> +	} else if (type == GATEWAY) {
> >>>> +		param_name = "gateway";
> >>>> +	} else {
> >>>> +		error = -EINVAL;
> >>>> +		goto cleanup;
> >>>> +	}
> >>>> +
> >>>> +	while (parse_ip_val_buffer(ip_string, &ip_offset, addr,
> >>>> +				   (MAX_IP_ADDR_SIZE * 2))) {
> >>>> +		ip_ver = ip_version_check(addr);
> >>>> +		if (ip_ver < 0)
> >>>> +			continue;
> >>>> +
> >>>> +		if ((ip_ver == IPV4 && ip_sec == IPV4) ||
> >>>> +		    (ip_ver == IPV6 && ip_sec == IPV6)) {
> >>>> +			if (((INET6_ADDRSTRLEN * MAX_IP_ENTRIES) - strlen(output_str)) >
> >>>> +			    (strlen(addr))) {
> >>>> +				strcat(output_str, addr);
> >>>> +				strcat(output_str, ",");
> >>>
> >>> Prefer strncat() here
> > Is this needed with the bound check above. I am trying to keep parity with the rest of the 
> > file.
> >>>
> <snip>
> 
> I missed this earlier because my comment was more of a general best practice comment.
> 
> Note that in the worst case where your bounds check (INET6_ADDRSTRLEN*MAX_IP_ENTRIES) - strlen(output_str) equals (strlen(addr) + 1),
> you will be adding strlen(addr)+1(","), and end up with no ASCII NUL '\0' delimiter.
> 
> If you're going to rely on the bounds check to ensure correctness, you'll need to correct that. Generally speaking, strncat would still
> be helpful in case the bounds check changes in the future.
> 
> Thanks,
> Easwar
got it. Thanks.