Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect netmask generation on certain architectures with the release 7.80 #1717

Closed
rfrohl opened this issue Aug 28, 2019 · 6 comments

Comments

@rfrohl
Copy link

commented Aug 28, 2019

Hi,
I am seeing a bug with the unit tests in the new 7.80 release. The test-addrset.sh fails for some specific tests on certain architectures: ppc64, ppc64le, s390x.

...
[   83s] cd test && (./test-addrset.sh&& ./test-cmdline-split&& ./test-uri&& ./test-wildcard&& echo "All tests passed.")
..
[   85s] PASS *.1-10,12.*.4-5,6,7                                                                                      
[   85s] FAIL "192.168.0.5 192.168.0.90 192.168.1.5 1.2.3.4" !=                                                                                                                                                                                
[   85s]      "192.168.0.5 192.168.0.90".                                                                              
[   85s] PASS 1.2.3.4/32  
..
[   86s] PASS 192.168.5,30,191.0/22                                                                                    
[   86s] FAIL "1:2::3 1:2::0 1:2::ff 1:2::1ff" !=                                                                      
[   86s]      "1:2::3 1:2::0 1:2::ff".                                                                                 
[   86s] PASS 1:2::0003/128

I was able to narrow it down to nbase_addrset.c:sockaddr_to_mask() line 503

mask[i] = ~((1 << (unmasked_bits - (32 * (4 - i)))) - 1);

Adding the following debug code:

printf("1: %u\n", (unmasked_bits - (32 * (4 - i))));
printf("2: %u\n", ((unmasked_bits - (32 * (4 - i))) - 1));
printf("3: %u\n", ((1 << (unmasked_bits - (32 * (4 - i)))) - 1));
printf("4: %u\n", ~((1 << (unmasked_bits - (32 * (4 - i)))) - 1));
mask[i] = ~((1 << (unmasked_bits - (32 * (4 - i)))) - 1);

and running the first failing test manual:

./addrset "192.168.0.0/24" <<EOF
192.168.0.5
192.168.0.90
192.168.1.5
1.2.3.4
EOF

I get for passing architectures:

1: 4294967272
2: 4294967271 
3: 255 
4: 4294967040

for failing ones:

1: 4294967272 
2: 4294967271 
3: 4294967295 
4: 0
@rfrohl

This comment has been minimized.

Copy link
Author

commented Aug 28, 2019

The core problem probably comes down to this (from ISO 9899:2011 6.5.7 Bit-wise shift operators):

The integer promotions are performed on each of the operands. The type of the result is that of
the promoted left operand. If the value of the right operand is negative or is greater than or
equal to the width of the promoted left operand, the behavior is undefined.

@rfrohl

This comment has been minimized.

Copy link
Author

commented Aug 28, 2019

looks like I found a solution, testing a patch on a few more architectures

rfrohl added a commit to rfrohl/nmap that referenced this issue Aug 28, 2019

@nnposter nnposter self-assigned this Aug 31, 2019

@nnposter nnposter added bug Nmap labels Aug 31, 2019

@nnposter

This comment has been minimized.

Copy link

commented Aug 31, 2019

Could you please test the following patch and report back?

--- a/nbase/nbase_addrset.c
+++ b/nbase/nbase_addrset.c
@@ -477,30 +477,32 @@
 
 static int sockaddr_to_mask (const struct sockaddr *sa, int bits, u32 *mask)
 {
-  s8 i;
-  int unmasked_bits = 0;
+  int i, k;
   if (bits >= 0) {
     if (sa->sa_family == AF_INET) {
-      unmasked_bits = 32 - bits;
+      bits += 96;
     }
 #ifdef HAVE_IPV6
     else if (sa->sa_family == AF_INET6) {
-      unmasked_bits = 128 - bits;
+      ; /* do nothing */
     }
 #endif
     else {
       return 0;
     }
   }
+  else
+    bits = 128;
+  k = bits / 32;
   for (i=0; i < 4; i++) {
-    if (unmasked_bits <= 32 * (3 - i)) {
+    if (i < k) {
       mask[i] = 0xffffffff;
     }
-    else if (unmasked_bits >= 32 * (4 - i)) {
+    else if (i > k) {
       mask[i] = 0;
     }
     else {
-      mask[i] = ~((1 << (unmasked_bits - (32 * (4 - i)))) - 1);
+      mask[i] = 0xfffffffe << (31 - bits % 32);
     }
   }
   return 1;
--- a/ncat/test/test-addrset.sh
+++ b/ncat/test/test-addrset.sh
@@ -208,6 +208,25 @@
 1:3::3
 EOF
 
+# IPv6 CIDR netmask.
+test_addrset "1:2::3:4:5/95" "1:2::3:4:5 1:2::2:0:0 1:2::3:ffff:ffff" <<EOF
+1:2::3:4:5
+1:2::1:ffff:ffff
+1:2::2:0:0
+1:2::3:ffff:ffff
+1:2::4:0:0
+1:3::3
+EOF
+
+# IPv6 CIDR netmask.
+test_addrset "11::2/15" "11::2:3:4:5 10::1 11:ffff:ffff:ffff:ffff:ffff:ffff:ffff" <<EOF
+11::2:3:4:5
+9:ffff:ffff:ffff:ffff:ffff:ffff:ffff
+10::1
+11:ffff:ffff:ffff:ffff:ffff:ffff:ffff
+12::0
+EOF
+
 # /128 netmask.
 test_addrset "1:2::0003/128" "1:2::3" <<EOF
 1:2::3
@rfrohl

This comment has been minimized.

Copy link
Author

commented Aug 31, 2019

Could you please test the following patch and report back?

Sure, but it will have to wait till Monday.

@rfrohl

This comment has been minimized.

Copy link
Author

commented Sep 2, 2019

Tested the new patch against the same architectures as the PR: x86_64, i586, ppc, ppc64, ppc64le, aarch64 and s390x.

Did not see any problems, so the patch looks good from what I can tell.

@nnposter

This comment has been minimized.

Copy link

commented Sep 3, 2019

I have committed the patch as r37726. Thank you for reporting the issue and pin-pointing the root cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.