Properly determine the host of a chess game on initialization

We previously used the length of the invite packet to determine
if we were the host of a chess game. This allowed a friend to
send an invite packet with a length of zero to trick you into
starting a game of chess with yourself as the host. The effect
of this was just a broken game in which neither peer could make
a move.

We now determine who the host is independently and in addition
do more thorough packet size enforcement

src/chat_commands.c

@@ -272,7 +272,7 @@ void cmd_game_join(WINDOW *window, ToxWindow *self, Tox *m, int argc, char (*arg
     uint8_t *data = Friends.list[self->num].game_invite.data;
     size_t length = Friends.list[self->num].game_invite.data_length;
 
-    int ret = game_initialize(self, m, type, id, data, length);
+    int ret = game_initialize(self, m, type, id, data, length, false);
 
     switch (ret) {
         case 0: {

src/game_base.c

@@ -203,7 +203,7 @@ static void game_toggle_pause(GameData *game)
     }
 }
 
-static int game_initialize_type(GameData *game, const uint8_t *data, size_t length)
+static int game_initialize_type(GameData *game, const uint8_t *data, size_t length, bool self_host)
 {
     int ret = -3;
 
@@ -219,7 +219,7 @@ static int game_initialize_type(GameData *game, const uint8_t *data, size_t leng
         }
 
         case GT_Chess: {
-            ret = chess_initialize(game, data, length);
+            ret = chess_initialize(game, data, length, self_host);
             break;
         }
 
@@ -237,7 +237,7 @@ static int game_initialize_type(GameData *game, const uint8_t *data, size_t leng
 }
 
 int game_initialize(const ToxWindow *parent, Tox *m, GameType type, uint32_t id, const uint8_t *multiplayer_data,
-                    size_t length)
+                    size_t length, bool self_host)
 {
     int max_x;
     int max_y;
@@ -293,7 +293,7 @@ int game_initialize(const ToxWindow *parent, Tox *m, GameType type, uint32_t id,
         return -4;
     }
 
-    int init_ret = game_initialize_type(game, multiplayer_data, length);
+    int init_ret = game_initialize_type(game, multiplayer_data, length, self_host);
 
     if (init_ret < 0) {
         game_init_abort(parent, self);
@@ -506,7 +506,7 @@ static int game_restart(GameData *game)
 
     game_clear_all_messages(game);
 
-    if (game_initialize_type(game, NULL, 0) == -1) {
+    if (game_initialize_type(game, NULL, 0, false) == -1) {
         return -1;
     }
 

src/game_base.h

@@ -214,10 +214,13 @@ void game_set_cb_on_packet(GameData *game, cb_game_on_packet *func, void *cb_dat
  * `type` must be a valid GameType.
  *
  * `id` should be a unique integer to indentify the game instance. If we're being invited to a game
- *   this identifier should be sent via the invite packet.
+ *  this identifier should be sent via the invite packet.
  *
- * if `multiplayer_data` is non-null this indicates that we accepted a game invite from a contact.
- *   The data contains any information we need to initialize the game state.
+ * if `multiplayer_data` is non-null it contains information that we received from the inviter
+ * necessary to initialize the game state.
+ *
+ * if `self_host` is true, the caller is the host of the game. If the game is not initialized from a
+ * friend's chat window this parameter has no effect.
  *
  * Return 0 on success.
  * Return -1 if screen is too small.
@@ -226,7 +229,7 @@ void game_set_cb_on_packet(GameData *game, cb_game_on_packet *func, void *cb_dat
  * Return -4 on other failure.
  */
 int game_initialize(const ToxWindow *self, Tox *m, GameType type, uint32_t id, const uint8_t *multiplayer_data,
-                    size_t length);
+                    size_t length, bool self_host);
 
 /*
  * Sets game window to `shape`.

src/game_chess.c

@@ -39,6 +39,12 @@
 #define CHESS_SQUARES (CHESS_BOARD_ROWS * CHESS_BOARD_COLUMNS)
 #define CHESS_MAX_MESSAGE_SIZE 64
 
+/* Packet sizes */
+#define CHESS_PACKET_SEND_MOVE_LENGTH 5
+#define CHESS_PACKET_RESIGN_LENGTH 1
+#define CHESS_PACKET_SEND_INVITE_LENGTH 2
+#define CHESS_PACKET_ACCEPT_INVITE_LENGTH 1
+
 typedef enum ChessPacketType {
     CHESS_PACKET_INIT_SEND_INVITE   = 0x01,
     CHESS_PACKET_INIT_ACCEPT_INVITE = 0x02,
@@ -1811,14 +1817,9 @@ void chess_cb_kill(GameData *game, void *cb_data)
  * Return 0 on success.
  * Return -1 on failure.
  */
-#define CHESS_PACKET_MOVE_SIZE 4
 static int chess_handle_opponent_move_packet(const GameData *game, ChessState *state, const uint8_t *data,
         size_t length)
 {
-    if (length < CHESS_PACKET_MOVE_SIZE || data == NULL) {
-        return -1;
-    }
-
     char from_l = data[0];
     uint8_t from_n = data[1];
     char to_l = data[2];
@@ -1899,6 +1900,11 @@ static void chess_cb_on_packet(GameData *game, const uint8_t *data, size_t lengt
 
     switch (type) {
         case CHESS_PACKET_INIT_ACCEPT_INVITE: {
+            if (length != CHESS_PACKET_ACCEPT_INVITE_LENGTH) {
+                fprintf(stderr, "Got invalid length invite accept packet (%zu)\n", length);
+                break;
+            }
+
             if (state->status == Initializing) {
                 state->status = Playing;
             }
@@ -1915,8 +1921,14 @@ static void chess_cb_on_packet(GameData *game, const uint8_t *data, size_t lengt
         }
 
         case CHESS_PACKET_MOVE_PIECE: {
+            if (length != CHESS_PACKET_SEND_MOVE_LENGTH) {
+                state->status = Resigned;
+                fprintf(stderr, "Got invalid length move packet (%zu)\n", length);
+                break;
+            }
+
             if (state->status == Playing) {
-                int ret = chess_handle_opponent_move_packet(game, state, data + 1, length - 1);
+                const int ret = chess_handle_opponent_move_packet(game, state, data + 1, length - 1);
 
                 if (ret != 0) {
                     state->status = Resigned;
@@ -2043,10 +2055,10 @@ static int chess_init_board(GameData *game, ChessState *state, bool self_is_whit
 
 static int chess_packet_send_resign(const GameData *game)
 {
-    uint8_t data[1];
+    uint8_t data[CHESS_PACKET_RESIGN_LENGTH];
     data[0] = CHESS_PACKET_RESIGN;
 
-    if (game_packet_send(game, data, 1, GP_Data) == -1) {
+    if (game_packet_send(game, data, sizeof(data), GP_Data) == -1) {
         return -1;
     }
 
@@ -2055,14 +2067,14 @@ static int chess_packet_send_resign(const GameData *game)
 
 static int chess_packet_send_move(const GameData *game, const Tile *from, const Tile *to)
 {
-    uint8_t data[5];
+    uint8_t data[CHESS_PACKET_SEND_MOVE_LENGTH];
     data[0] = CHESS_PACKET_MOVE_PIECE;
     data[1] = from->chess_coords.L;
     data[2] = from->chess_coords.N;
     data[3] = to->chess_coords.L;
     data[4] = to->chess_coords.N;
 
-    if (game_packet_send(game, data, 5, GP_Data) == -1) {
+    if (game_packet_send(game, data, sizeof(data), GP_Data) == -1) {
         return -1;
     }
 
@@ -2071,11 +2083,11 @@ static int chess_packet_send_move(const GameData *game, const Tile *from, const
 
 static int chess_packet_send_invite(const GameData *game, bool self_is_white)
 {
-    uint8_t data[2];
+    uint8_t data[CHESS_PACKET_SEND_INVITE_LENGTH];
     data[0] = CHESS_PACKET_INIT_SEND_INVITE;
     data[1] = self_is_white ? Black : White;
 
-    if (game_packet_send(game, data, 2, GP_Invite) == -1) {
+    if (game_packet_send(game, data, sizeof(data), GP_Invite) == -1) {
         return -1;
     }
 
@@ -2084,17 +2096,17 @@ static int chess_packet_send_invite(const GameData *game, bool self_is_white)
 
 static int chess_packet_send_accept(const GameData *game)
 {
-    uint8_t data[1];
+    uint8_t data[CHESS_PACKET_ACCEPT_INVITE_LENGTH];
     data[0] = CHESS_PACKET_INIT_ACCEPT_INVITE;
 
-    if (game_packet_send(game, data, 1, GP_Data) == -1) {
+    if (game_packet_send(game, data, sizeof(data), GP_Data) == -1) {
         return -1;
     }
 
     return 0;
 }
 
-int chess_initialize(GameData *game, const uint8_t *init_data, size_t length)
+int chess_initialize(GameData *game, const uint8_t *init_data, size_t length, bool self_host)
 {
     if (game_set_window_shape(game, GW_ShapeSquare) == -1) {
         return -1;
@@ -2106,14 +2118,17 @@ int chess_initialize(GameData *game, const uint8_t *init_data, size_t length)
         return -3;
     }
 
-    bool self_is_host = false;
-    bool self_is_white = false;
+    bool self_is_white = rand() % 2 == 0;
 
-    if (length == 0) {
-        self_is_host = true;
-        self_is_white = rand() % 2 == 0;
-    } else {
-        if (length < 2 || init_data[0] != CHESS_PACKET_INIT_SEND_INVITE) {
+    if (!self_host) {
+        if (length != CHESS_PACKET_SEND_INVITE_LENGTH) {
+            fprintf(stderr, "Tried to join a game with invalid game data of length %zu\n", length);
+            free(state);
+            return -2;
+        }
+
+        if (init_data[0] != CHESS_PACKET_INIT_SEND_INVITE) {
+            fprintf(stderr, "Failed to join chess game: invalid invite data\n");
             free(state);
             return -2;
         }
@@ -2141,7 +2156,7 @@ int chess_initialize(GameData *game, const uint8_t *init_data, size_t length)
     state->other.can_castle_ks = true;
     state->other.can_castle_qs = true;
 
-    if (self_is_host) {
+    if (self_host) {
         if (chess_packet_send_invite(game, self_is_white) == -1) {
             free(state);
             return -2;

src/game_chess.h

@@ -28,17 +28,17 @@
 /*
  * Initializes chess game state.
  *
- * If `init_data` is non-null, this indicates that we were invited to the game.
+ * If `self_host` is false, this indicates that we were invited to the game.
  *
- * If we're the inviter, we send an invite packet after initialization. If we're the
- * invitee, we send a handshake response packet to the inviter.
+ * `init_data` of length `length` is the game data sent to us by the inviter
+ * needed to start the game.
  *
  * Return 0 on success.
  * Return -1 if window is too small.
  * Return -2 on network related error.
  * Return -3 on other error.
  */
-int chess_initialize(GameData *game, const uint8_t *init_data, size_t length);
+int chess_initialize(GameData *game, const uint8_t *init_data, size_t length, bool self_host);
 
 #endif // GAME_CHESS
 

src/global_commands.c

@@ -363,7 +363,7 @@ void cmd_game(WINDOW *window, ToxWindow *self, Tox *m, int argc, char (*argv)[MA
     }
 
     uint32_t id = rand();
-    int ret = game_initialize(self, m, type, id, NULL, 0);
+    int ret = game_initialize(self, m, type, id, NULL, 0, true);
 
     switch (ret) {
         case 0: {