inet_ntop bug in 1.1.19

Submitted by Philip Homburg on June 4, 2018, 1:57 p.m.

Details

Message ID a7e32d88-2b5f-e9f9-8108-7fdb3958a130@ripe.net
State New
Series "inet_ntop bug in 1.1.19"
Headers show

Commit Message

Philip Homburg June 4, 2018, 1:57 p.m.
(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.
/*
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);
}

Patch hide | download patch | download mbox

--- 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) {

Comments

Laurent Bercot June 4, 2018, 2:23 p.m.
>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
Timo Teras June 4, 2018, 6:39 p.m.
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
Rich Felker June 5, 2018, 12:37 a.m.
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