excludefile order matters if smaller range is included in bigger range

[nv1t]

Changing the order of the IP ranges inside the excludefile changes the outcome. Having a smaller IP-Range which is included in a bigger IP-Range and both defined in the excludefile, might excludes everything, if small IP-Range is in front of the big IP-Range.
This can be a single IP Address as well instead of an IP-Range.

It only happens if the smaller range is included in the bigger range.

To Reproduce
Running: nmap --excludefile exclude.list 192.168.0.0/16 with the following two different excludefiles:

Small Range in front of Big Range:

-> % cat exclude.list.doesnotwork
192.168.1.64/25
192.168.1.0/24
-> % nmap --excludefile exclude.list.doesnotwork 192.168.0.0/16
Starting Nmap 7.80 ( https://nmap.org ) at 2021-02-25 10:28 CET
WARNING: No targets were specified, so 0 hosts scanned.
Nmap done: 0 IP addresses (0 hosts up) scanned in 0.04 seconds

Small Range after Big Range

-> % cat exclude.list.works
192.168.1.0/24
192.168.1.64/25
-> % nmap --excludefile exclude.list.works 192.168.0.0/16 -vvv
Starting Nmap 7.80 ( https://nmap.org ) at 2021-02-25 10:37 CET
Initiating Ping Scan at 10:37
Scanning 4096 hosts [2 ports/host]
...

Expected behavior
It shouldn't matter which option the IP-Ranges are inside the excludefile.

Version info (please complete the following information):

• OS: Linux 5.8.0-43-generic

-> % nmap --version
Nmap version 7.80 ( https://nmap.org )
Platform: x86_64-pc-linux-gnu
Compiled with: liblua-5.3.3 openssl-1.1.1d nmap-libssh2-1.8.2 libz-1.2.11 libpcre-8.39 libpcap-1.9.1 nmap-libdnet-1.12 ipv6
Compiled without:
Available nsock engines: epoll poll select

Same behaviour with: Nmap version 7.91

[dmiller-nmap]

Thanks for reporting this. I'm hunting down the problem, which seems to be an edge case I missed when implementing the exclusion trie merging code. Specifically, it seems that if the address part of a CIDR spec falls within a CIDR spec already in the trie, we end up doing the wrong thing, probably when we assume this means the earlier spec is the larger one. For example, if you use a different pair of specs such that the base address of the larger spec does not fall within the smaller one, the bug doesn't trigger: 192.168.1.128/25 and 192.168.1.0/24, for instance.

[dmiller-nmap]

Bug is in this section:

   if (this->mask[i] > mask[i]) {
        /* broader mask, truncate this one */
        this->mask[i] = mask[i];
        for (; i < 4; i++) {
          this->mask[i] = 0;
        }

We correctly take the new, broader mask for this u32 (in the case of IPv4, that's the entire mask), but then we set each u32 in the mask to 0. We should skip the current one since we already grabbed the correct value.

I'm actually rewriting a few portions of this to do the work in trie_split() instead, since that is going to be a bit more efficient. Will update this issue when I push a fix.