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

Fixed undefined behavior in netmask generation #1717 #1718

Closed
wants to merge 1 commit into from

Conversation

@rfrohl
Copy link

commented Aug 28, 2019

Tested this change on x86_64, i586, ppc, ppc64, ppc64le, aarch64 and s390x. Unit tests passed successfully on these architectures.

Also played around with the test binary (addrset) a bit and could not find any obvious problems with the change.

@nnposter
Copy link

left a comment

Over at #1717 I have proposed an alternate patch, which also disposes of the rather complex arithmetic in the original code.

@@ -500,7 +500,7 @@ static int sockaddr_to_mask (const struct sockaddr *sa, int bits, u32 *mask)
mask[i] = 0;
}
else {
mask[i] = ~((1 << (unmasked_bits - (32 * (4 - i)))) - 1);
mask[i] = ~(0xffffffff % (1 << unmasked_bits));

This comment has been minimized.

Copy link
@nnposter

nnposter Aug 31, 2019

This code suffers from a similar issue as the original one: There is no guaranteed outcome when unmasked_bits exceeds 31. As an example:

#include <stdio.h>
void main ()
{
int m=32;
printf("%08x\n%08x\n",(0xFFFFFFFF<<(m-1)<<1),(0xFFFFFFFF<<m));
}

When compiled with GCC on Ubuntu 18.04 x64:

$ ./a.out 
00000000
ffffffff

This comment has been minimized.

Copy link
@rfrohl

rfrohl Aug 31, 2019

Author

I assumed that it would hit another branch if unmasked_bits is a multiple of 32. But I my understanding of how mask is used in the rest of the code is limited, so I might miss something here.

Will test the other patch from the ticket on Monday

@nnposter nnposter self-assigned this Aug 31, 2019

@nnposter nnposter added bug Nmap labels Aug 31, 2019

@nmap-bot nmap-bot closed this in 9e8852a Sep 3, 2019

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.