From: Benjamin Auder <benjamin.auder@somewhere>
Date: Wed, 26 Feb 2020 08:45:11 +0000 (+0100)
Subject: Add basic secutiry when updating moves on server
X-Git-Url: https://git.auder.net/%7B%7B%20asset%28%27mixstore/css/store/current/%7B%7B%20pkg.url%20%7D%7D?a=commitdiff_plain;h=fb68b0c2e3dae0be3e1f55b9516070c2731cf8d9;p=vchess.git

Add basic secutiry when updating moves on server
---

diff --git a/server/models/Game.js b/server/models/Game.js
index ae91ac94..21ece2f1 100644
--- a/server/models/Game.js
+++ b/server/models/Game.js
@@ -208,7 +208,7 @@ const GameModel =
   },
 
   // obj can have fields move, chat, fen, drawOffer and/or score + message
-  update: function(id, obj)
+  update: function(id, obj, cb)
   {
     db.parallelize(function() {
       let query =
@@ -235,14 +235,27 @@ const GameModel =
         query += modifs + " WHERE id = " + id;
         db.run(query);
       }
+      let wrongMoveIndex = false;
       if (obj.move)
       {
-        const m = obj.move;
+        // Security: only update moves if index is right
         query =
-          "INSERT INTO Moves (gid, squares, played, idx) VALUES " +
-          "(" + id + ",?," + m.played + "," + m.idx + ")";
-        db.run(query, JSON.stringify(m.squares));
+          "SELECT MAX(idx) AS maxIdx " +
+          "FROM Moves " +
+          "WHERE gid = " + id;
+        db.get(query, (err,ret) => {
+          const m = obj.move;
+          if (!ret.maxIdx || ret.maxIdx + 1 == m.idx) {
+            query =
+              "INSERT INTO Moves (gid, squares, played, idx) VALUES " +
+              "(" + id + ",?," + m.played + "," + m.idx + ")";
+            db.run(query, JSON.stringify(m.squares));
+            cb(null);
+          }
+          else cb({errmsg:"Wrong move index"});
+        });
       }
+      else cb(null);
       if (obj.chat)
       {
         query =
diff --git a/server/routes/games.js b/server/routes/games.js
index c8de00dc..1b788cb9 100644
--- a/server/routes/games.js
+++ b/server/routes/games.js
@@ -63,20 +63,21 @@ router.put("/games", access.logged, access.ajax, (req,res) => {
     GameModel.getPlayers(gid, (err,players) => {
       if (players.some(p => p.uid == req.userId))
       {
-        GameModel.update(gid, obj);
-        if (obj.move || obj.score)
-        {
-          // Notify opponent if he enabled notifications:
-          const oppid = players[0].uid == req.userId
-            ? players[1].uid
-            : players[0].uid;
-          const messagePrefix = obj.move
-            ? "New move in game: "
-            : "Game ended: ";
-          UserModel.tryNotify(oppid,
-            messagePrefix + params.siteURL + "/#/game/" + gid);
-        }
-        res.json({});
+        GameModel.update(gid, obj, (err) => {
+          if (!err && (obj.move || obj.score))
+          {
+            // Notify opponent if he enabled notifications:
+            const oppid = players[0].uid == req.userId
+              ? players[1].uid
+              : players[0].uid;
+            const messagePrefix = obj.move
+              ? "New move in game: "
+              : "Game ended: ";
+            UserModel.tryNotify(oppid,
+              messagePrefix + params.siteURL + "/#/game/" + gid);
+          }
+          res.json(err || {});
+        });
       }
     });
   }