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);

Reply via email to