Need stricter, OS-independent IPv4/IPv6 address parser

laanwj commented at 2:58 pm on January 28, 2016: member

I’m not sure if this will be solved by libevent, it could be, in any case there needs to be an issue for this: currently CNetAddr (and to a lesser extent, CService) farm out address parsing to the OS, which causes it (at the discretion of the OS) to accept incomplete as well as otherwise invalid addresses. @EthanHeilman perfectly illustrates this in #7291:

0BOOST_CHECK(CNetAddr("1.1.1").ToString() == "1.1.0.1");
1BOOST_CHECK(CNetAddr("1.1").ToString() == "1.0.0.1");
2BOOST_CHECK(CNetAddr("1111").ToString() == "0.0.4.87");

Parsing incomplete addresses as they were complete ones causes confusion when specifying ban masks. For example a user may think that 1.1.1, as it is accepted, maps to 1.1.1.0/24. Mapping to 1.1.0.1 is just strange. It should simply reject these.

Also, CNetaddr is not supposed to accept a port! but parses this as IPv6 any addr:

0BOOST_CHECK(CNetAddr("250.2.2.2:7777").ToString() == "::");
1BOOST_CHECK(CNetAddr("1.1.1.1:1").ToString() == CNetAddr("2.2.2:1").ToString());

These cases too should result an address address that is not valid (!IsValid()).

laanwj added the label P2P on Jan 28, 2016

laanwj commented at 12:29 pm on January 29, 2016: member

@theuni Do you know if libevent has built-in IPv4/IPv6 address parsing, or does it suffer from the same issue, that it’s completely OS dependent?

EthanHeilman commented at 4:05 pm on January 29, 2016: contributor

This problem results from the relaxed parsing standard OS provided IPv4 libraries perform for backwards compatibility reasons. As Zalewski points out in The Tangled Web:

Although the RFC permits only canonical notations for IP addresses, standard C libraries used by most applications are much more relaxed, accepting noncanonical IPv4 addresses that octal, decimal and hexadecimal notation. [..] As a result the following options are recognized as equivalent: https://127.0.0.1/, http://0x7f.1/, http://017700000001/.

Libevent has a switch to use its own very strict IP parser rather than the standard relaxed ones which are causing this issue:

 0int
 1evutil_inet_pton(int af, const char *src, void *dst)
 2{
 3#if defined(EVENT__HAVE_INET_PTON) && !defined(USE_INTERNAL_PTON)
 4    return inet_pton(af, src, dst);
 5#else
 6    if (af == AF_INET) {
 7        unsigned a,b,c,d;
 8        char more;
 9        struct in_addr *addr = dst;
10        if (sscanf(src, "%u.%u.%u.%u%c", &a,&b,&c,&d,&more) != 4)
11            return 0;
12        if (a > 255) return 0;
13        if (b > 255) return 0;
14        if (c > 255) return 0;
15        if (d > 255) return 0;
16        addr->s_addr = htonl((a<<24) | (b<<16) | (c<<8) | d);
17        return 1;
18#ifdef AF_INET6
19    } else if (af == AF_INET6) {

https://github.com/libevent/libevent/blob/1c17cfdd2bea34089f303a2dcbcb05fcacf46cd6/evutil.c#L1933

We should switch to using the libevent parser.

laanwj closed this on Nov 22, 2019

MarcoFalke locked this on Dec 16, 2021

Need stricter, OS-independent IPv4/IPv6 address parser #7435