On Sun, Dec 06, 2015 at 07:37:27PM +0100, Theo Buehler wrote: > The current implementation of the selection of a random sequence of > ports in nc -r suffers from modulo bias and a biased shuffling > procedure. Use arc4random_uniform() and the Fisher-Yates shuffle > instead.
daniel@ pointed out that it might be nicer to combine initialization and shuffling, as in ip_id.c, so here's a second version of the patch. The patch is bigger, but the resulting code looks tighter. Index: usr.bin/nc/netcat.c =================================================================== RCS file: /cvs/src/usr.bin/nc/netcat.c,v retrieving revision 1.144 diff -u -p -r1.144 netcat.c --- usr.bin/nc/netcat.c 23 Nov 2015 01:23:56 -0000 1.144 +++ usr.bin/nc/netcat.c 6 Dec 2015 20:53:20 -0000 @@ -1289,6 +1289,23 @@ build_ports(char *p) lo = cp; } + if (rflag) { + /* + * Initialize portlist[] with a random permutation. + * Based on Knuth, as in ip_randomid() in ip_id.c. + */ + for (x = 0; x <= hi - lo; x++) { + cp = arc4random_uniform(x + 1); + portlist[x] = portlist[cp]; + portlist[cp] = calloc(1, PORT_MAX_LEN); + if (portlist[cp] == NULL) + err(1, NULL); + snprintf(portlist[cp], PORT_MAX_LEN, "%d", + x + lo); + } + return; + } + /* Load ports sequentially. */ for (cp = lo; cp <= hi; cp++) { portlist[x] = calloc(1, PORT_MAX_LEN); @@ -1296,19 +1313,6 @@ build_ports(char *p) err(1, NULL); snprintf(portlist[x], PORT_MAX_LEN, "%d", cp); x++; - } - - /* Randomly swap ports. */ - if (rflag) { - int y; - char *c; - - for (x = 0; x <= (hi - lo); x++) { - y = (arc4random() & 0xFFFF) % (hi - lo); - c = portlist[x]; - portlist[x] = portlist[y]; - portlist[y] = c; - } } } else { hi = strtonum(p, 1, PORT_MAX, &errstr);