CSubNet: fix CIDR netmask calculation and match() function #6185

pull jonasschnelli wants to merge 1 commits into bitcoin:master from jonasschnelli:2015/05/CSubNet changing 2 files +23 −5

jonasschnelli commented at 7:37 PM on May 25, 2015: contributor

While working on the RPC ban function, i have detected the the CSubNet CIDR netmask calculations as well as the CSubNet::Match() function are buggy.

Edit: fixes #6179
CSubNet: fix CIDR netmask calculation and match() function 4f6444a5c3

in src/netbase.cpp:None in 4f6444a5c3

1270 | +                        {
1271 | +                            netmask[15-i] = netmask[15-i] << shift;
1272 | +                            break;
1273 | +                        }
1274 | +                        shift-=8;
1275 | +                    }

jonasschnelli commented at 7:41 PM on May 25, 2015:

This is not so one-linerish. My cleverness is limited. I hope someone finds the time to do this nicer.

laanwj commented at 6:34 AM on May 26, 2015:

Trying a different take on this

in src/test/netbase_tests.cpp:None in 4f6444a5c3

 108 | @@ -109,6 +109,11 @@ BOOST_AUTO_TEST_CASE(subnet_test)
 109 |      BOOST_CHECK(CSubNet("1.2.3.0/24") != CSubNet("1.2.4.0/255.255.255.0"));
 110 |      BOOST_CHECK(CSubNet("1.2.3.0/24").Match(CNetAddr("1.2.3.4")));
 111 |      BOOST_CHECK(!CSubNet("1.2.2.0/24").Match(CNetAddr("1.2.3.4")));
 112 | +
 113 | +    BOOST_CHECK(CSubNet("1.2.2.1/24").Match(CNetAddr("1.2.2.4")));

laanwj commented at 5:48 AM on May 26, 2015:

So to be clear: these tests fail without this change? I doubt you need as many changes to fix this. Particularly the change to match appears unnecessarily complex. It is not simply a matter of bitwise AND-ing the addr with netmask after parsing?

laanwj commented at 7:03 AM on May 26, 2015: member

See #6186
jonasschnelli commented at 7:10 AM on May 26, 2015: contributor

Closing in favor of #6186.
jonasschnelli closed this on May 26, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors

jonasschnelli

laanwj

Linked (view graph)

#6179 CSubNet::Match(): calculation error