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.

Sign up for GitHub

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

Jump to bottom

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

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

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

rfrohl opened this issue Aug 28, 2019 · 6 comments
Assignees
@nnposter
Labels
bug Nmap

Comments

@rfrohl
Copy link

rfrohl 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
Copy link
Author

rfrohl 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
Copy link
Author

rfrohl commented Aug 28, 2019 •
edited

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
@rfrohl
Fixed undefined behavior in netmask generation nmap#1717
146e21d
@nnposter nnposter self-assigned this Aug 31, 2019
@nnposter
Copy link

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
Copy link
Author

rfrohl commented Aug 31, 2019

Could you please test the following patch and report back?

Sure, but it will have to wait till Monday.

@rfrohl
Copy link
Author

rfrohl 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
Copy link

nnposter commented Sep 3, 2019

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

@nnposter nnposter mentioned this issue Sep 13, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nnposter nnposter

Labels
bug Nmap
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rfrohl @nnposter