On Mon, Jan 18, 2021 at 10:59:36AM +0100, Florian Obser wrote:
> On Thu, Jan 14, 2021 at 09:34:49AM -0500, Brad House wrote:
> > While working on debugging an issue reported in c-ares, I noticed some test
> > case failures in inet_net_pton(). I haven't evaluated most of them, however
> > one stood out:
> >
> > ../test/ares-test-internal.cc:150: Failure
> > Expected: -1
> > To be equal to: inet_net_pton(24,"12:34::ff",&a6,sizeof(a6) - 1)
> > Which is: 128
> >
> > This test expects inet_net_pton() to fail because the buffer provided was of
> > insufficient size.
> >
> > I'm not all that familiar with OpenBSD itself, a quick google search turned
> > up this as the possible source:
> > https://github.com/libressl-portable/openbsd/blob/master/src/lib/libc/net/inet_net_pton.c
> >
> > The 'size' parameter to inet_net_pton_ipv6() is *never* used:
> > https://github.com/libressl-portable/openbsd/blob/master/src/lib/libc/net/inet_net_pton.c#L206
> >
> > I believe this failure ends up later causing other test failures due to
> > memory corruption in the c-ares test cases.
> >
> >
>
> Thanks for the report, this is a potential fix:
I would have expected others to chime in, but since that didn't happen:
ok tb
>
> diff --git lib/libc/net/inet_net_pton.c lib/libc/net/inet_net_pton.c
> index 2aaeac4048a..d7cc75e0092 100644
> --- lib/libc/net/inet_net_pton.c
> +++ lib/libc/net/inet_net_pton.c
> @@ -205,9 +205,10 @@ inet_net_pton_ipv4(const char *src, u_char *dst, size_t
> size)
> static int
> inet_net_pton_ipv6(const char *src, u_char *dst, size_t size)
> {
> - int ret;
> - int bits;
> - char
> buf[sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:255:255:255:255/128")];
> + struct in6_addr in6;
> + int ret;
> + int bits;
> + char buf[INET6_ADDRSTRLEN + sizeof("/128")];
> char *sep;
> const char *errstr;
>
> @@ -220,18 +221,24 @@ inet_net_pton_ipv6(const char *src, u_char *dst, size_t
> size)
> if (sep != NULL)
> *sep++ = '\0';
>
> - ret = inet_pton(AF_INET6, buf, dst);
> + ret = inet_pton(AF_INET6, buf, &in6);
> if (ret != 1)
> return (-1);
>
> if (sep == NULL)
> - return 128;
> + bits = 128;
> + else {
> + bits = strtonum(sep, 0, 128, &errstr);
> + if (errstr) {
> + errno = EINVAL;
> + return (-1);
> + }
> + }
>
> - bits = strtonum(sep, 0, 128, &errstr);
> - if (errstr) {
> - errno = EINVAL;
> + if ((bits + 7) / 8 > size) {
> + errno = EMSGSIZE;
> return (-1);
> }
> -
> - return bits;
> + memcpy(dst, &in6.s6_addr, size);
> + return (bits);
> }
>
>
> --
> I'm not entirely sure you are real.
>
