From: Benjamin Auder <benjamin.auder@somewhere>
Date: Sat, 8 Feb 2020 11:22:30 +0000 (+0100)
Subject: Sanitize more
X-Git-Url: https://git.auder.net/images/doc/html/%7B%7B%20path%28%27fos_user_security_login%27%29%20%7D%7D?a=commitdiff_plain;h=99b7a14c6e01c53a49459c8d4681acf6abe635d8;p=vchess.git

Sanitize more
---

diff --git a/server/routes/challenges.js b/server/routes/challenges.js
index 28103fce..a7adcf5c 100644
--- a/server/routes/challenges.js
+++ b/server/routes/challenges.js
@@ -7,6 +7,8 @@ const UserModel = require("../models/User"); //for name check
 const params = require("../config/parameters");
 
 router.get("/challenges", (req,res) => {
+  if (!req.query["uid"].match(/^[0-9]+$/))
+    res.json({errmsg: "Bad user ID"});
   ChallengeModel.getByUser(req.query["uid"], (err,challenges) => {
     res.json(err || {challenges:challenges});
   });
@@ -46,6 +48,8 @@ router.post("/challenges", access.logged, access.ajax, (req,res) => {
 
 router.delete("/challenges", access.logged, access.ajax, (req,res) => {
   const cid = req.query.id;
+  if (!cid.match(/^[0-9]+$/))
+    res.json({errmsg: "Bad challenge ID"});
   ChallengeModel.safeRemove(cid, req.userId, err => {
     res.json(err || {}); //TODO: just "return err" because is empty if no errors
   });
diff --git a/server/routes/games.js b/server/routes/games.js
index bef8bf5e..42325856 100644
--- a/server/routes/games.js
+++ b/server/routes/games.js
@@ -10,7 +10,7 @@ const params = require("../config/parameters");
 router.post("/games", access.logged, access.ajax, (req,res) => {
   const gameInfo = req.body.gameInfo;
   if (!Array.isArray(gameInfo.players) ||
-    !gameInfo.players.some(p => p.id == req.userId))
+    gameInfo.players.every(p => p.id != req.userId))
   {
     return res.json({errmsg: "Cannot start someone else's game"});
   }
@@ -43,6 +43,8 @@ router.get("/games", access.ajax, (req,res) => {
   const gameId = req.query["gid"];
   if (!!gameId)
   {
+    if (!gameId.match(/^[0-9]+$/))
+      return res.json({errmsg: "Wrong game ID"});
     GameModel.getOne(gameId, (err,game) => {
       access.checkRequest(res, err, game, "Game not found", () => {
         res.json({game: game});
@@ -53,6 +55,8 @@ router.get("/games", access.ajax, (req,res) => {
   {
     // Get by (non-)user ID:
     const userId = req.query["uid"];
+    if (!userId.match(/^[0-9]+$/))
+      return res.json({errmsg: "Wrong user ID"});
     const excluded = !!req.query["excluded"];
     GameModel.getByUser(userId, excluded, (err,games) => {
       if (!!err)
diff --git a/server/routes/messages.js b/server/routes/messages.js
index cd93b9fe..d96cbfa2 100644
--- a/server/routes/messages.js
+++ b/server/routes/messages.js
@@ -9,10 +9,10 @@ router.post("/messages", (req,res,next) => {
   if (!req.xhr)
     return res.json({errmsg: "Unauthorized access"});
   const from = req.body["email"];
-  const subject = req.body["subject"];
-  const body = req.body["content"];
+  // Replace potential newline characters in subject
+  const subject = req.body["subject"].replace(/\r?\n|\r/g, " ");
+  const body = req.body["content"]; //TODO: sanitize? Why? How?
 
-  // TODO: sanitize ?
   mailer(from, params.mail.contact, subject, body, err => {
     if (!!err)
       return res.json({errmsg:err});
diff --git a/server/routes/users.js b/server/routes/users.js
index ab139eb6..abe987cd 100644
--- a/server/routes/users.js
+++ b/server/routes/users.js
@@ -20,6 +20,8 @@ router.get("/whoami", access.ajax, (req,res) => {
   const anonymous = {name:"", email:"", id:0, notify:false};
   if (!req.cookies.token)
     return callback(anonymous);
+  if (!req.cookies.token.match(/^[a-z0-9]+$/))
+    return res.json({errmsg: "Bad token"});
   UserModel.getOne("sessionToken", req.cookies.token, function(err, user) {
     if (!!err || !user)
       callback(anonymous);
@@ -31,6 +33,8 @@ 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"];
+  if (!ids.match(/^([0-9]+,?)+$/)) //NOTE: slightly too permissive
+    return res.json({errmsg: "Bad IDs array"});
   UserModel.getByIds(ids, (err,users) => {
     if (!!err)
       return res.json({errmsg: err.toString()});
@@ -90,6 +94,8 @@ router.get('/sendtoken', access.unlogged, access.ajax, (req,res) => {
 });
 
 router.get('/authenticate', access.unlogged, access.ajax, (req,res) => {
+  if (!req.query.token.match(/^[a-z0-9]+$/))
+    return res.json({errmsg: "Bad token"});
   UserModel.getOne("loginToken", req.query.token, (err,user) => {
     access.checkRequest(res, err, user, "Invalid token", () => {
       // If token older than params.tokenExpire, do nothing