inet_ntop bug in 1.1.19

inet_ntop bug in 1.1.19

[Philip Homburg]

[-- Attachment #1: Type: text/plain, Size: 389 bytes --]

(Please CC me on any replies, I'm not subscribed to the list)

inet_ntop doesn't conform to RFC 5952 (A Recommendation for IPv6 Address
Text Representation).

I attached a test program to demonstrate the issue and a patch:
$ cc inet_ntop_test.c musl-1.1.19/src/network/inet_ntop.c
$ ./a.out
Section 4.2.2 test failed: got 2001:db8::1:1:1:1:1, expected
2001:db8:0:1:1:1:1:1
Found 1 error.


[-- Attachment #2: inet_ntop_test.c --]
[-- Type: text/plain, Size: 2273 bytes --]

/*
Check if inet_ntop is according to RFC 5952
*/

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <arpa/inet.h>
#include <netinet/in.h>
#include <sys/socket.h>

/* Section 4.1: Handling Leading Zeros in a 16-Bit Field */
struct in6_addr s41_input = { {
			0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00,
			0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 } };
char *s41_str = "2001:db8::1";

/* Section 4.2.1: Shorten as Much as Possible */
struct in6_addr s421_input = { {
			0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00,
			0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x01 } };
char *s421_str = "2001:db8::2:1";

/* Section 4.2.2: Handling One 16-Bit 0 Field */
struct in6_addr s422_input = { {
			0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x01,
			0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01 } };
char *s422_str = "2001:db8:0:1:1:1:1:1";

/* Section 4.2.3a: Choice in Placement of "::" */
struct in6_addr s423a_input = { {
			0x20, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
			0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 } };
char *s423a_str = "2001:0:0:1::1";

/* Section 4.2.3b: Choice in Placement of "::" */
struct in6_addr s423b_input = { {
			0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00,
			0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 } };
char *s423b_str = "2001:db8::1:0:0:1";

struct tests {
	char *label;
	struct in6_addr *addrp;
	char **strp;
} tests[] = {
	{ "Section 4.1", &s41_input, &s41_str },
	{ "Section 4.2.1", &s421_input, &s421_str },
	{ "Section 4.2.2", &s422_input, &s422_str },
	{ "Section 4.2.3a", &s423a_input, &s423a_str },
	{ "Section 4.2.3b", &s423b_input, &s423b_str },
	{ NULL, NULL, NULL }
};

int main(void)
{
	int i, errors;
	char buf[INET6_ADDRSTRLEN];

	errors= 0;
	for (i= 0; tests[i].label != NULL; i++)
	{
		if (inet_ntop(AF_INET6, tests[i].addrp, buf, sizeof(buf)) ==
			NULL)
		{
			fprintf(stderr, "inet_ntop failed for test %s: %s\n",
				tests[i].label, strerror(errno));
			exit(1);
		}
		if (strcmp(buf, *tests[i].strp) != 0)
		{
			fprintf(stderr, "%s test failed: got %s, expected %s\n",
				tests[i].label, buf, *tests[i].strp);
			errors++;
		}
	}
	if (errors)
	{
		fprintf(stderr, "Found %d error%s.\n",
			errors, errors == 1 ? "" : "s");
		exit(1);
	}
	exit(0);
}
	

[-- Attachment #3: inet_ntop.c.patch --]
[-- Type: text/plain, Size: 327 bytes --]

--- musl-1.1.19/src/network/inet_ntop.c	2018-02-22 19:39:19.000000000 +0100
+++ inet_ntop.c	2018-06-04 15:51:23.192207973 +0200
@@ -34,6 +34,7 @@
 		for (i=best=0, max=2; buf[i]; i++) {
 			if (i && buf[i] != ':') continue;
 			j = strspn(buf+i, ":0");
+			if (j<4) continue;
 			if (j>max) best=i, max=j;
 		}
 		if (max>2) {

Re: inet_ntop bug in 1.1.19

[Laurent Bercot]

>inet_ntop doesn't conform to RFC 5952 (A Recommendation for IPv6 
>Address
>Text Representation).
>
>I attached a test program to demonstrate the issue and a patch:
>$ cc inet_ntop_test.c musl-1.1.19/src/network/inet_ntop.c
>$ ./a.out
>Section 4.2.2 test failed: got 2001:db8::1:1:1:1:1, expected
>2001:db8:0:1:1:1:1:1

  https://tools.ietf.org/html/rfc5952#section-4.2.1 says:
  "The use of the symbol "::" MUST be used to its maximum capability."

  2001:db8::1:1:1:1:1 is the correct canonical text representation.

--
  Laurent

Re: inet_ntop bug in 1.1.19

[Timo Teras]

On Mon, 04 Jun 2018 14:23:55 +0000
"Laurent Bercot" <ska-dietlibc@skarnet.org> wrote:

> >inet_ntop doesn't conform to RFC 5952 (A Recommendation for IPv6 
> >Address
> >Text Representation).
> >
> >I attached a test program to demonstrate the issue and a patch:
> >$ cc inet_ntop_test.c musl-1.1.19/src/network/inet_ntop.c
> >$ ./a.out
> >Section 4.2.2 test failed: got 2001:db8::1:1:1:1:1, expected
> >2001:db8:0:1:1:1:1:1  
> 
>   https://tools.ietf.org/html/rfc5952#section-4.2.1 says:
>   "The use of the symbol "::" MUST be used to its maximum capability."
> 
>   2001:db8::1:1:1:1:1 is the correct canonical text representation.

The following section 4.2.2  says:

4.2.2.  Handling One 16-Bit 0 Field

   The symbol "::" MUST NOT be used to shorten just one 16-bit 0 field.
   For example, the representation 2001:db8:0:1:1:1:1:1 is correct, but
   2001:db8::1:1:1:1:1 is not correct.

Looks like the test case is taken directly from this.

Timo

Re: inet_ntop bug in 1.1.19

[Rich Felker]

On Mon, Jun 04, 2018 at 09:39:21PM +0300, Timo Teras wrote:
> On Mon, 04 Jun 2018 14:23:55 +0000
> "Laurent Bercot" <ska-dietlibc@skarnet.org> wrote:
> 
> > >inet_ntop doesn't conform to RFC 5952 (A Recommendation for IPv6 
> > >Address
> > >Text Representation).
> > >
> > >I attached a test program to demonstrate the issue and a patch:
> > >$ cc inet_ntop_test.c musl-1.1.19/src/network/inet_ntop.c
> > >$ ./a.out
> > >Section 4.2.2 test failed: got 2001:db8::1:1:1:1:1, expected
> > >2001:db8:0:1:1:1:1:1  
> > 
> >   https://tools.ietf.org/html/rfc5952#section-4.2.1 says:
> >   "The use of the symbol "::" MUST be used to its maximum capability."
> > 
> >   2001:db8::1:1:1:1:1 is the correct canonical text representation.
> 
> The following section 4.2.2  says:
> 
> 4.2.2.  Handling One 16-Bit 0 Field
> 
>    The symbol "::" MUST NOT be used to shorten just one 16-bit 0 field.
>    For example, the representation 2001:db8:0:1:1:1:1:1 is correct, but
>    2001:db8::1:1:1:1:1 is not correct.
> 
> Looks like the test case is taken directly from this.

Lovely -- 4.2.1 and 4.2.2 are outright conflicting. I suppose you're
expected to interpret 4.2.1 as "maximum capability subject to the
nonsensical additional constraint below".

In any case, 4.2.2 probably makes things prettier to read even if it
does take an extra character.

I checked and only RFC 2373 is actually normative for inet_pton (per
POSIX), but it doesn't contradict anything in RFC 5952 or provide any
preferred conventions for reverse mapping, so I think it's safe to
adopt the rule in 4.2.2 above.

Sound ok?

Rich

Re: inet_ntop bug in 1.1.19

[Rich Felker]

I think Philip somehow got un-Cc'd and missed further discussion:

On Mon, Jun 04, 2018 at 08:37:24PM -0400, Rich Felker wrote:
> On Mon, Jun 04, 2018 at 09:39:21PM +0300, Timo Teras wrote:
> > On Mon, 04 Jun 2018 14:23:55 +0000
> > "Laurent Bercot" <ska-dietlibc@skarnet.org> wrote:
> > 
> > > >inet_ntop doesn't conform to RFC 5952 (A Recommendation for IPv6 
> > > >Address
> > > >Text Representation).
> > > >
> > > >I attached a test program to demonstrate the issue and a patch:
> > > >$ cc inet_ntop_test.c musl-1.1.19/src/network/inet_ntop.c
> > > >$ ./a.out
> > > >Section 4.2.2 test failed: got 2001:db8::1:1:1:1:1, expected
> > > >2001:db8:0:1:1:1:1:1  
> > > 
> > >   https://tools.ietf.org/html/rfc5952#section-4.2.1 says:
> > >   "The use of the symbol "::" MUST be used to its maximum capability."
> > > 
> > >   2001:db8::1:1:1:1:1 is the correct canonical text representation.
> > 
> > The following section 4.2.2  says:
> > 
> > 4.2.2.  Handling One 16-Bit 0 Field
> > 
> >    The symbol "::" MUST NOT be used to shorten just one 16-bit 0 field.
> >    For example, the representation 2001:db8:0:1:1:1:1:1 is correct, but
> >    2001:db8::1:1:1:1:1 is not correct.
> > 
> > Looks like the test case is taken directly from this.
> 
> Lovely -- 4.2.1 and 4.2.2 are outright conflicting. I suppose you're
> expected to interpret 4.2.1 as "maximum capability subject to the
> nonsensical additional constraint below".
> 
> In any case, 4.2.2 probably makes things prettier to read even if it
> does take an extra character.
> 
> I checked and only RFC 2373 is actually normative for inet_pton (per
> POSIX), but it doesn't contradict anything in RFC 5952 or provide any
> preferred conventions for reverse mapping, so I think it's safe to
> adopt the rule in 4.2.2 above.
> 
> Sound ok?

Arthur Jones subsequently submitted a slightly simpler patch which I'm
applying. Let me know if anything still seems wrong. It now passes the
test case.

Rich