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:
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.
