Invert test to avoid undefined behavior (#58)

* Invert test to avoid undefined behavior

Otherwise, when i == MAX_PLAYERS, we end up accessing an element beyond the end
of the array. Found with Frama-C.

* Add patch for bug

* Add detail about bug

challenges/WhackJack/CMakeLists.txt

@@ -1,5 +1,5 @@
 set( SERVICE_ID "00065" )
 set( AUTHOR_ID "CROMU" )
 add_compile_options( -O0 -g -DENABLE_BUFFERED_PRINTF )
-set( VULN_COUNT "1" )
+set( VULN_COUNT "2" )
 buildCB()

challenges/WhackJack/README.md

@@ -36,3 +36,8 @@ This vulnerability is easy to find and very easy to fix.  Exploitation is hard g
 	Proving Vulnerability 1: hard
 	Fixing Vulnerability 1: easy
 
+### Unmarked Bugs/Vulnerabilities
+
+Vulnerability 2 was unmarked in the original challenge, and it is more of a bug/undefined behavior than something that can be exploited. The fix is to reorder the tests to avoid undefined behavior. Otherwise, when `i == MAX_PLAYERS`, we end up accessing an element beyond the end of the array. Found with Frama-C.
+
+No PoV is provided.

challenges/WhackJack/src/player.c

@@ -39,7 +39,15 @@ int method;
 
 	i = 0;
 
+#ifdef PATCHED_2
+
+	while (i < MAX_PLAYERS && playerList[i].player_name[0] != 0)
+
+#else
+
 	while (playerList[i].player_name[0] != 0 && i < MAX_PLAYERS) 
+
+#endif
 		++i;
 
 	if (i == MAX_PLAYERS) {