From: Florian Obser <florian@openbsd.org>
Subject: Re: inet_aton(3): fix long standing bug
To: tech@openbsd.org
Date: Mon, 26 Aug 2024 21:15:31 +0200

This might have been funnier if I hadn't typo'ed the reference.

2.10.  Robustness Principle

  TCP implementations should follow a general principle of robustness:
  be conservative in what you do, be liberal in what you accept from
  others.

I.e. the first instance of Postel's law.


On 2024-08-26 17:47 +02, Florian Obser <florian@openbsd.org> wrote:
> It occurred to me that inet_aton(3) does not conform to section 2.3 of
> RFC 761. I believe this is the case since 4.3bsd.
>
> Now, we have replaced most uses of inet_aton(3) in base, but I think we
> should still fix it, after all, config parsers in ports might still use
> inet_aton(3) and we don't want config files to fail to load.
>
> OK?
>
> diff --git lib/libc/net/inet_addr.c lib/libc/net/inet_addr.c
> index 62d46ad664f..5dcecb43769 100644
> --- lib/libc/net/inet_addr.c
> +++ lib/libc/net/inet_addr.c
> @@ -85,7 +85,9 @@ inet_aton(const char *cp, struct in_addr *addr)
>  	char c;
>  	u_int parts[4];
>  	u_int *pp = parts;
> +	char *op;
>  
> +	op = cp;
>  	c = *cp;
>  	for (;;) {
>  		/*
> @@ -94,7 +96,7 @@ inet_aton(const char *cp, struct in_addr *addr)
>  		 * 0x=hex, 0=octal, isdigit=decimal.
>  		 */
>  		if (!isdigit((unsigned char)c))
> -			return (0);
> +			goto lasttry;
>  		val = 0; base = 10;
>  		if (c == '0') {
>  			c = *++cp;
> @@ -125,7 +127,7 @@ inet_aton(const char *cp, struct in_addr *addr)
>  			 *	a.b	(with b treated as 24 bits)
>  			 */
>  			if (pp >= parts + 3)
> -				return (0);
> +				goto lasttry;
>  			*pp++ = val;
>  			c = *++cp;
>  		} else
> @@ -136,7 +138,7 @@ inet_aton(const char *cp, struct in_addr *addr)
>  	 */
>  	if (c != '\0' &&
>  	    (!isascii((unsigned char)c) || !isspace((unsigned char)c)))
> -		return (0);
> +		goto lasttry;
>  	/*
>  	 * Concoct the address according to
>  	 * the number of parts specified.
> @@ -145,31 +147,40 @@ inet_aton(const char *cp, struct in_addr *addr)
>  	switch (n) {
>  
>  	case 0:
> -		return (0);		/* initial nondigit */
> +		goto lasttry;		/* initial nondigit */
>  
>  	case 1:				/* a -- 32 bits */
>  		break;
>  
>  	case 2:				/* a.b -- 8.24 bits */
>  		if ((val > 0xffffff) || (parts[0] > 0xff))
> -			return (0);
> +			goto lasttry;
>  		val |= parts[0] << 24;
>  		break;
>  
>  	case 3:				/* a.b.c -- 8.8.16 bits */
>  		if ((val > 0xffff) || (parts[0] > 0xff) || (parts[1] > 0xff))
> -			return (0);
> +			goto lasttry;
>  		val |= (parts[0] << 24) | (parts[1] << 16);
>  		break;
>  
>  	case 4:				/* a.b.c.d -- 8.8.8.8 bits */
>  		if ((val > 0xff) || (parts[0] > 0xff) || (parts[1] > 0xff) || (parts[2] > 0xff))
> -			return (0);
> +			goto lasttry;
>  		val |= (parts[0] << 24) | (parts[1] << 16) | (parts[2] << 8);
>  		break;
>  	}
> +
>  	if (addr)
>  		addr->s_addr = htonl(val);
>  	return (1);
> +
> +lasttry:
> +	val = (in_addr_t)op;
> +
> +	if (addr)
> +		addr->s_addr = htonl(val);
> +	return (1);
> +
>  }
>  DEF_WEAK(inet_aton);
>
> -- 
>
> In my defence, I have been left unsupervised.
>

-- 
In my defence, I have been left unsupervised.