From: Benjamin Auder Date: Sat, 1 Feb 2020 00:45:45 +0000 (+0100) Subject: Sanitize inputs on server side X-Git-Url: https://git.auder.net/doc/html/vendor/app_dev.php?a=commitdiff_plain;h=58e7b94e6e1a8d5721b9211b45c40e65fc13f600;p=vchess.git Sanitize inputs on server side --- diff --git a/server/db/create.sql b/server/db/create.sql index 4f7b547c..a3930725 100644 --- a/server/db/create.sql +++ b/server/db/create.sql @@ -32,7 +32,6 @@ create table Challenges ( foreign key (vid) references Variants(id) ); --- NOTE: no need for a "created" field, it's deduced from first move playing time create table Games ( id integer primary key, vid integer, diff --git a/server/models/Challenge.js b/server/models/Challenge.js index 01de6b54..fa0407dc 100644 --- a/server/models/Challenge.js +++ b/server/models/Challenge.js @@ -1,4 +1,5 @@ var db = require("../utils/database"); +const UserModel = require("./User"); /* * Structure: @@ -21,6 +22,8 @@ const ChallengeModel = return "Wrong characters in time control"; if (!c.fen.match(/^[a-zA-Z0-9, /-]*$/)) return "Bad FEN string"; + if (!!c.to) + return UserModel.checkNameEmail({name: c.to}); return ""; }, @@ -53,7 +56,7 @@ const ChallengeModel = }); }, - // all challenges except where target is defined and not me + // All challenges except where target is defined and not me getByUser: function(uid, cb) { db.serialize(function() { diff --git a/server/models/Game.js b/server/models/Game.js index 0ac822e8..5b225ce6 100644 --- a/server/models/Game.js +++ b/server/models/Game.js @@ -1,4 +1,5 @@ var db = require("../utils/database"); +const UserModel = require("./User"); /* * Structure table Games: @@ -9,6 +10,7 @@ var db = require("../utils/database"); * timeControl: string * score: varchar (result) * created: datetime + * drawOffer: boolean * * Structure table Players: * gid: ref game id @@ -31,6 +33,24 @@ var db = require("../utils/database"); const GameModel = { + checkGameInfo: function(g) { + if (!g.id.toString().match(/^[0-9]+$/)) + return "Wrong game ID"; + if (!g.vid.toString().match(/^[0-9]+$/)) + return "Wrong variant ID"; + if (!g.vname.match(/^[a-zA-Z0-9]+$/)) + return "Wrong variant name"; + if (!g.timeControl.match(/^[0-9dhms +]+$/)) + return "Wrong characters in time control"; + if (!g.fen.match(/^[a-zA-Z0-9, /-]*$/)) + return "Bad FEN string"; + if (g.players.length != 2) + return "Need exactly 2 players"; + if (g.players.some(p => !p.id.toString().match(/^[0-9]+$/))) + return "Wrong characters in player ID"; + return ""; + }, + create: function(vid, fen, timeControl, players, cb) { db.serialize(function() { @@ -149,6 +169,29 @@ const GameModel = }); }, + checkGameUpdate: function(obj) + { + // Check all that is possible (required) in obj: + if (!!obj.move) + { + if (!obj.move.played.toString().match(/^[0-9]+$/)) + return "Wrong move played time"; + if (!obj.move.idx.toString().match(/^[0-9]+$/)) + return "Wrong move index"; + } + if (!!obj.fen && !obj.fen.match(/^[a-zA-Z0-9, /-]*$/)) + return "Wrong FEN string"; + if (!!obj.score && !obj.score.match(/^[012?*\/-]+$/)) + return "Wrong characters in score"; + if (!!obj.chat) + { + if (!obj.chat.sid.match(/^[a-zA-Z0-9]+$/)) + return "Wrong user SID"; + return UserModel.checkNameEmail({name: obj.chat.name}); + } + return ""; + }, + // obj can have fields move, chat, fen, drawOffer and/or score update: function(id, obj) { @@ -159,7 +202,7 @@ const GameModel = let modifs = ""; if (!!obj.message) modifs += "message = message || ' ' || '" + obj.message + "',"; - if (!!obj.drawOffer) + if ([true,false].includes(obj.drawOffer)) modifs += "drawOffer = " + obj.drawOffer + ","; if (!!obj.fen) modifs += "fen = '" + obj.fen + "',"; @@ -176,17 +219,16 @@ const GameModel = const m = obj.move; query = "INSERT INTO Moves (gid, squares, played, idx) VALUES " + - "(" + id + ",'" + JSON.stringify(m.squares) + "'," - + m.played + "," + m.idx + ")"; - db.run(query); + "(" + id + ",?," + m.played + "," + m.idx + ")"; + db.run(query, JSON.stringify(m.squares)); } if (!!obj.chat) { query = "INSERT INTO Chats (gid, msg, name, sid, added) VALUES " + - "(" + id + ",'" + obj.chat.msg + "','" + obj.chat.name + - "','" + obj.chat.sid + "'," + Date.now() + ")"; - db.run(query); + "(" + id + ",?,'" + obj.chat.name + "','" + + obj.chat.sid + "'," + Date.now() + ")"; + db.run(query, obj.chat.msg); } }); }, diff --git a/server/models/User.js b/server/models/User.js index ee4b0566..c0516156 100644 --- a/server/models/User.js +++ b/server/models/User.js @@ -33,6 +33,7 @@ const UserModel = if (!o.email.match(/^[\w.+-]+@[\w.+-]+$/)) return "Bad characters in email"; } + return ""; //NOTE: not required, but more consistent... (?!) }, // NOTE: parameters are already cleaned (in controller), thus no sanitization here @@ -89,8 +90,9 @@ const UserModel = }, // Set session token only if empty (first login) - // TODO: weaker security (but avoid to re-login everywhere after each logout) - trySetSessionToken: function(uid, cb) + // NOTE: weaker security (but avoid to re-login everywhere after each logout) + // TODO: option would be to reset all tokens periodically, e.g. every 3 months + trySetSessionToken: function(uid, cb) { // Also empty the login token to invalidate future attempts db.serialize(function() { diff --git a/server/routes/games.js b/server/routes/games.js index a444acdf..e9d9ab46 100644 --- a/server/routes/games.js +++ b/server/routes/games.js @@ -9,11 +9,22 @@ var params = require("../config/parameters"); // From main hall, start game between players 0 and 1 router.post("/games", access.logged, access.ajax, (req,res) => { const gameInfo = req.body.gameInfo; - if (!gameInfo.players.some(p => p.id == req.userId)) + if (!Array.isArray(gameInfo.players) || + !gameInfo.players.some(p => p.id == req.userId)) + { return res.json({errmsg: "Cannot start someone else's game"}); + } const cid = req.body.cid; + // Check all entries of gameInfo + cid: + let error = GameModel.checkGameInfo(gameInfo); + if (!error) + { + if (!cid.toString().match(/^[0-9]+$/)) + error = "Wrong challenge ID"; + } + if (!!error) + return res.json({errmsg:error}); ChallengeModel.remove(cid); - const fen = req.body.fen; GameModel.create( gameInfo.vid, gameInfo.fen, gameInfo.timeControl, gameInfo.players, (err,ret) => { @@ -55,7 +66,13 @@ router.get("/games", access.ajax, (req,res) => { // TODO: if newmove fail, takeback in GUI router.put("/games", access.logged, access.ajax, (req,res) => { const gid = req.body.gid; - const obj = req.body.newObj; + let error = ""; + if (!gid.toString().match(/^[0-9]+$/)) + error = "Wrong game ID"; + const obj = req.body.newObj; + error = GameModel.checkGameUpdate(obj); + if (!!error) + return res.json({errmsg: error}); GameModel.update(gid, obj, (err) => { if (!!err) return res.json(err); diff --git a/server/routes/messages.js b/server/routes/messages.js index 02ddec55..b3c158eb 100644 --- a/server/routes/messages.js +++ b/server/routes/messages.js @@ -8,7 +8,6 @@ const params = require(__dirname.replace("/routes", "/config/parameters")); router.post("/messages", (req,res,next) => { if (!req.xhr) return res.json({errmsg: "Unauthorized access"}); - console.log(req.body); const from = req.body["email"]; const subject = req.body["subject"]; const body = req.body["content"]; diff --git a/server/routes/users.js b/server/routes/users.js index 163dc301..1d553dba 100644 --- a/server/routes/users.js +++ b/server/routes/users.js @@ -7,6 +7,7 @@ var genToken = require("../utils/tokenGenerator"); var access = require("../utils/access"); var params = require("../config/parameters"); +// NOTE: this method is safe because the sessionToken must be guessed router.get("/whoami", access.ajax, (req,res) => { const callback = (user) => { return res.json({ @@ -27,6 +28,7 @@ router.get("/whoami", access.ajax, (req,res) => { }); }); +// NOTE: this method is safe because only IDs and names are returned router.get("/users", access.ajax, (req,res) => { const ids = req.query["ids"]; UserModel.getByIds(ids, (err,users) => { diff --git a/server/utils/mailer.js b/server/utils/mailer.js index 8a059da2..86e647c2 100644 --- a/server/utils/mailer.js +++ b/server/utils/mailer.js @@ -26,10 +26,11 @@ module.exports = function(from, to, subject, body, cb) // Setup email data with unicode symbols const mailOptions = { - from: from, //note: some SMTP serves might forbid this + from: params.mail.noreply, to: to, subject: subject, text: body, + replyTo: from, }; // Send mail with the defined transport object