New $SRANDOM feature uses a set seed on Cygwin

[JohnoKing]

The implementation of arc4random on Cygwin appears to use a set seed, or otherwise lacks adequate entropy. Reproducer:

$ (ulimit -t unlimited; echo $SRANDOM); echo $SRANDOM
2844622778
2844622778

$ (ulimit -t unlimited; echo $SRANDOM; echo $SRANDOM); echo $SRANDOM; echo $SRANDOM
4065042319
37404502
4065042319
37404502

As a consequence of this bug, two of the regression tests for $RANDOM now fail on Cygwin because the seed is sourced from arc4random (also slightly unrelated, but the folder name is i386 when it should be i386-64):

$ bin/shtests -p variables
#### Regression-testing /home/ksh/arch/cygwin.i386/bin/ksh ####
test variables begins at 2024-01-25+15:33:49
        variables.sh[50]: FAIL: Test 1: $RANDOM seed in subshell doesn't change (both results are 29935)
        variables.sh[91]: FAIL: Test 4: $RANDOM seed in ( simple_command & ) doesn't change (both results are 29935)
test variables failed at 2024-01-25+15:33:59 with exit code 2 [ 213 tests 2 errors ]
Total errors: 2

[McDutchie]

Well, that's atrocious. But this is really Cygwin's bug and should be reported to them, right?

In the meantime we could block Cygwin and use the implementation we ship instead; not trusting their implementation by default seems like good policy at this point.

But won't we get into a name conflict with the OS-shipped version then? We may need to add some macro voodoo to src/lib/libast/features/map.c.

[JohnoKing]

It has been reported to the Cygwin mailing list: https://cygwin.com/pipermail/cygwin/2024-January/255245.html

[JohnoKing]

This bug has been fixed upstream in Cygwin commit 030a7625. That fix is only set to be released with Cygwin 3.5 (which drops Win7/8 support), so a feature test for older versions of Cygwin should still be added.

[McDutchie]

Congrats on getting it fixed. :-)

Yes, I'll try to turn your reproducer into a feature test.

[McDutchie]

@JohnoKing, could you test this patch on Cygwin? Thanks!

diff --git a/src/lib/libast/features/random b/src/lib/libast/features/random
index 459cde022..11e7d633c 100644
--- a/src/lib/libast/features/random
+++ b/src/lib/libast/features/random
@@ -19,3 +19,59 @@ tst note{ does the system have a source of secure randomness }end run{
 	00000)	exit 1 ;;  # result: "no"
 	esac
 }end
+
+# test for bug in Cygwin < 3.5
+# https://cygwin.com/pipermail/cygwin/2024-January/255245.html
+# https://github.com/ksh93/ksh/issues/711
+tst note{ does arc4random(3) become predictable after forking }end output{
+	#include <fcntl.h>
+	#include <stdlib.h>
+	#include <string.h>
+	#include <unistd.h>
+	#include <sys/wait.h>
+	#define PATHSIZE 1024
+	int main(int argc, char** argv)
+	{
+		char		file[PATHSIZE], *t = file, *f = argv[0];
+		int		fd, result;
+		pid_t		child;
+		uint32_t	r[3], r2[3];
+		/* create data file */
+		while (*t = *f++ && t < &file[PATHSIZE-4])
+			t++;
+		*t++ = '.', *t++ = 'D', *t = 0;
+		if ((fd = open(file, O_CREAT|O_TRUNC|O_WRONLY, 0666)) < 0)
+			return 128;
+		/* get arc4random() data from fork */
+		child = fork();
+		if (child == -1)
+			return close(fd), remove(file), 128;
+		else if (child == 0)
+		{
+			r[0] = arc4random(), r[1] = arc4random(), r[2] = arc4random();
+			write(fd, r, sizeof r);
+			return 0;
+		}
+		waitpid(child, NULL, 0);
+		close(fd);
+		/* get arc4random() data from main */
+		r[0] = arc4random(), r[1] = arc4random(), r[2] = arc4random();
+		/* compare */
+		if ((fd = open(file, O_RDWR)) < 0)
+			return remove(file), 128;
+		result = read(fd, r2, sizeof r);
+		close(fd);
+		remove(file);
+		if (result != sizeof r)
+			return 128;
+		result = !memcmp(r, r2, sizeof r);
+		if(result)
+		{
+			printf("/* native arc4random(3) broken on this system */\n");
+			printf("#undef _lib_arc4random\n");
+			printf("#undef _lib_arc4random_buf\n");
+			printf("#undef _lib_arc4random_uniform\n");
+		}
+		return !result;
+	}
+}end

[JohnoKing]

I have confirmed that the patch works as expected on Cygwin 3.4.10.

[McDutchie]

Briliant, thanks!