added CSRF protection using csurf express middleware, fixes #455

package-lock.json

@@ -2012,6 +2012,26 @@
       "integrity": "sha1-ojD2T1aDEOFJgAmUB5DsmVRbyn4=",
       "dev": true
     },
+    "csrf": {
+      "version": "3.0.6",
+      "resolved": "https://registry.npmjs.org/csrf/-/csrf-3.0.6.tgz",
+      "integrity": "sha1-thEg3c7q/JHnbtUxO7XAsmZ7cQo=",
+      "requires": {
+        "rndm": "1.2.0",
+        "tsscmp": "1.0.5",
+        "uid-safe": "2.1.4"
+      },
+      "dependencies": {
+        "uid-safe": {
+          "version": "2.1.4",
+          "resolved": "https://registry.npmjs.org/uid-safe/-/uid-safe-2.1.4.tgz",
+          "integrity": "sha1-Otbzg2jG1MjHXsF2I/t5qh0HHYE=",
+          "requires": {
+            "random-bytes": "1.0.0"
+          }
+        }
+      }
+    },
     "css-select": {
       "version": "1.2.0",
       "resolved": "https://registry.npmjs.org/css-select/-/css-select-1.2.0.tgz",
@@ -2041,6 +2061,34 @@
         "cssom": "0.3.4"
       }
     },
+    "csurf": {
+      "version": "1.9.0",
+      "resolved": "https://registry.npmjs.org/csurf/-/csurf-1.9.0.tgz",
+      "integrity": "sha1-SdLGkl/87Ht95VlZfBU/pTM2QTM=",
+      "requires": {
+        "cookie": "0.3.1",
+        "cookie-signature": "1.0.6",
+        "csrf": "3.0.6",
+        "http-errors": "1.5.1"
+      },
+      "dependencies": {
+        "http-errors": {
+          "version": "1.5.1",
+          "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-1.5.1.tgz",
+          "integrity": "sha1-eIwNLB3iyBuebowBhDtrl+uSB1A=",
+          "requires": {
+            "inherits": "2.0.3",
+            "setprototypeof": "1.0.2",
+            "statuses": "1.5.0"
+          }
+        },
+        "setprototypeof": {
+          "version": "1.0.2",
+          "resolved": "https://registry.npmjs.org/setprototypeof/-/setprototypeof-1.0.2.tgz",
+          "integrity": "sha1-gaVSFB7BBLiOic44MQOtXGZWTQg="
+        }
+      }
+    },
     "cuint": {
       "version": "0.2.2",
       "resolved": "https://registry.npmjs.org/cuint/-/cuint-0.2.2.tgz",
@@ -10450,6 +10498,11 @@
         "glob": "7.1.3"
       }
     },
+    "rndm": {
+      "version": "1.2.0",
+      "resolved": "https://registry.npmjs.org/rndm/-/rndm-1.2.0.tgz",
+      "integrity": "sha1-8z/pz7Urv9UgqhgyO8ZdsRCht2w="
+    },
     "run-async": {
       "version": "2.3.0",
       "resolved": "https://registry.npmjs.org/run-async/-/run-async-2.3.0.tgz",
@@ -11700,6 +11753,11 @@
       "integrity": "sha512-4krF8scpejhaOgqzBEcGM7yDIEfi0/8+8zDRZhNZZ2kjmHJ4hv3zCbQWxoJGz1iw5U0Jl0nma13xzHXcncMavQ==",
       "dev": true
     },
+    "tsscmp": {
+      "version": "1.0.5",
+      "resolved": "https://registry.npmjs.org/tsscmp/-/tsscmp-1.0.5.tgz",
+      "integrity": "sha1-fcSjOvcVgatDN9qR2FylQn69mpc="
+    },
     "tunnel-agent": {
       "version": "0.4.3",
       "resolved": "https://registry.npmjs.org/tunnel-agent/-/tunnel-agent-0.4.3.tgz",

package.json

@@ -27,6 +27,7 @@
     "cls-hooked": "4.2.2",
     "commonmark": "0.28.1",
     "cookie-parser": "1.4.4",
+    "csurf": "^1.9.0",
     "dayjs": "1.8.11",
     "debug": "4.1.1",
     "ejs": "2.6.1",

src/public/javascripts/desktop.js

@@ -98,7 +98,8 @@ $(document).on("click", "button[data-help-page]", e => {
 $("#logout-button").toggle(!utils.isElectron());
 
 $("#logout-button").click(() => {
-    const $logoutForm = $('<form action="logout" method="POST">');
+    const $logoutForm = $('<form action="logout" method="POST">')
+                            .append($(`<input type="hidden" name="_csrf" value="${glob.csrfToken}"/>`));
 
     $("body").append($logoutForm);
     $logoutForm.submit();

src/public/javascripts/services/server.js

@@ -1,4 +1,3 @@
-import protectedSessionHolder from './protected_session_holder.js';
 import utils from './utils.js';
 import infoService from "./info.js";
 
@@ -7,7 +6,8 @@ function getHeaders() {
     // so hypothetical protectedSessionId becomes protectedsessionid on the backend
     // also avoiding using underscores instead of dashes since nginx filters them out by default
     return {
-        'trilium-source-id': glob.sourceId
+        'trilium-source-id': glob.sourceId,
+        'x-csrf-token': glob.csrfToken
     };
 }
 

src/routes/custom.js

@@ -4,6 +4,8 @@ const fileUploadService = require('./api/file_upload');
 const scriptService = require('../services/script');
 
 function register(router) {
+    // explicitly no CSRF middleware since it's meant to allow integration from external services
+
     router.all('/custom/:path*', async (req, res, next) => {
         // express puts content after first slash into 0 index element
         const path = req.params.path + req.params[0];

src/routes/index.js

@@ -12,6 +12,7 @@ async function index(req, res) {
     const view = req.cookies['trilium-device'] === 'mobile' ? 'mobile' : 'desktop';
 
     res.render(view, {
+        csrfToken: req.csrfToken(),
         theme: options.theme,
         leftPaneMinWidth: parseInt(options.leftPaneMinWidth),
         leftPaneWidthPercent: parseInt(options.leftPaneWidthPercent),

src/routes/routes.js

@@ -38,6 +38,9 @@ const auth = require('../services/auth');
 const cls = require('../services/cls');
 const sql = require('../services/sql');
 const protectedSessionService = require('../services/protected_session');
+const csurf = require('csurf');
+
+const csrfMiddleware = csurf({ cookie: true });
 
 function apiResultHandler(req, res, result) {
     // if it's an array and first element is integer then we consider this to be [statusCode, response] format
@@ -59,7 +62,7 @@ function apiResultHandler(req, res, result) {
 }
 
 function apiRoute(method, path, routeHandler) {
-    route(method, path, [auth.checkApiAuth], routeHandler, apiResultHandler);
+    route(method, path, [auth.checkApiAuth, csrfMiddleware], routeHandler, apiResultHandler);
 }
 
 function route(method, path, middleware, routeHandler, resultHandler, transactional = true) {
@@ -95,10 +98,10 @@ const GET = 'get', POST = 'post', PUT = 'put', DELETE = 'delete';
 const uploadMiddleware = multer.single('upload');
 
 function register(app) {
-    route(GET, '/', [auth.checkAuth], indexRoute.index);
+    route(GET, '/', [auth.checkAuth, csrfMiddleware], indexRoute.index);
     route(GET, '/login', [auth.checkAppInitialized], loginRoute.loginPage);
     route(POST, '/login', [], loginRoute.login);
-    route(POST, '/logout', [auth.checkAuth], loginRoute.logout);
+    route(POST, '/logout', [csrfMiddleware, auth.checkAuth], loginRoute.logout);
     route(GET, '/setup', [auth.checkAppNotInitialized], setupRoute.setupPage);
 
     apiRoute(GET, '/api/tree', treeApiRoute.getTree);
@@ -129,9 +132,9 @@ function register(app) {
     apiRoute(PUT, '/api/notes/:noteId/clone-after/:afterBranchId', cloningApiRoute.cloneNoteAfter);
 
     route(GET, '/api/notes/:branchId/export/:type/:format/:version/:exportId', [auth.checkApiAuthOrElectron], exportRoute.exportBranch);
-    route(POST, '/api/notes/:parentNoteId/import', [auth.checkApiAuthOrElectron, uploadMiddleware], importRoute.importToBranch, apiResultHandler);
+    route(POST, '/api/notes/:parentNoteId/import', [auth.checkApiAuthOrElectron, uploadMiddleware, csrfMiddleware], importRoute.importToBranch, apiResultHandler);
 
-    route(POST, '/api/notes/:parentNoteId/upload', [auth.checkApiAuthOrElectron, uploadMiddleware],
+    route(POST, '/api/notes/:parentNoteId/upload', [auth.checkApiAuthOrElectron, uploadMiddleware, csrfMiddleware],
         filesRoute.uploadFile, apiResultHandler);
 
     route(GET, '/api/notes/:noteId/download', [auth.checkApiAuthOrElectron], filesRoute.downloadFile);
@@ -148,7 +151,7 @@ function register(app) {
     apiRoute(GET, '/api/attributes/values/:attributeName', attributesRoute.getValuesForAttribute);
 
     route(GET, '/api/images/:noteId/:filename', [auth.checkApiAuthOrElectron], imageRoute.returnImage);
-    route(POST, '/api/images', [auth.checkApiAuthOrElectron, uploadMiddleware], imageRoute.uploadImage, apiResultHandler);
+    route(POST, '/api/images', [auth.checkApiAuthOrElectron, uploadMiddleware, csrfMiddleware], imageRoute.uploadImage, apiResultHandler);
 
     apiRoute(GET, '/api/recent-changes', recentChangesApiRoute.getRecentChanges);
 
@@ -176,6 +179,7 @@ function register(app) {
     apiRoute(POST, '/api/recent-notes', recentNotesRoute.addRecentNote);
     apiRoute(GET, '/api/app-info', appInfoRoute.getAppInfo);
 
+    // group of services below are meant to be executed from outside
     route(GET, '/api/setup/status', [], setupApiRoute.getStatus, apiResultHandler);
     route(POST, '/api/setup/new-document', [auth.checkAppNotInitialized], setupApiRoute.setupNewDocument, apiResultHandler);
     route(POST, '/api/setup/sync-from-server', [auth.checkAppNotInitialized], setupApiRoute.setupSyncFromServer, apiResultHandler, false);
@@ -188,7 +192,7 @@ function register(app) {
 
     apiRoute(POST, '/api/cleanup/cleanup-unused-images', cleanupRoute.cleanupUnusedImages);
     // VACUUM requires execution outside of transaction
-    route(POST, '/api/cleanup/vacuum-database', [auth.checkApiAuthOrElectron], cleanupRoute.vacuumDatabase, apiResultHandler, false);
+    route(POST, '/api/cleanup/vacuum-database', [auth.checkApiAuthOrElectron, csrfMiddleware], cleanupRoute.vacuumDatabase, apiResultHandler, false);
 
     apiRoute(POST, '/api/script/exec', scriptRoute.exec);
     apiRoute(POST, '/api/script/run/:noteId', scriptRoute.run);
@@ -196,6 +200,7 @@ function register(app) {
     apiRoute(GET, '/api/script/bundle/:noteId', scriptRoute.getBundle);
     apiRoute(GET, '/api/script/relation/:noteId/:relationName', scriptRoute.getRelationBundles);
 
+    // no CSRF since this is called from android app
     route(POST, '/api/sender/login', [], senderRoute.login, apiResultHandler);
     route(POST, '/api/sender/image', [auth.checkSenderToken, uploadMiddleware], senderRoute.uploadImage, apiResultHandler);
     route(POST, '/api/sender/note', [auth.checkSenderToken], senderRoute.saveNote, apiResultHandler);

src/views/desktop.ejs

@@ -237,7 +237,8 @@
         activeDialog: null,
         sourceId: '<%= sourceId %>',
         maxSyncIdAtLoad: <%= maxSyncIdAtLoad %>,
-        instanceName: '<%= instanceName %>'
+        instanceName: '<%= instanceName %>',
+        csrfToken: '<%= csrfToken %>'
     };
     window.appCssNoteIds = <%- JSON.stringify(appCssNoteIds) %>;
 </script>

src/views/mobile.ejs

@@ -68,7 +68,9 @@
 
     <div class="dropdown-menu dropdown-menu-sm" id="context-menu-container"></div>
 
-    <form action="logout" id="logout-form" method="POST" style="display: none;"></form>
+    <form action="logout" id="logout-form" method="POST" style="display: none;">
+        <input type="hidden" name="_csrf" value="<%= csrfToken %>"/>
+    </form>
 </div>
 
 <script type="text/javascript">
@@ -78,7 +80,8 @@
         activeDialog: null,
         sourceId: '<%= sourceId %>',
         maxSyncIdAtLoad: <%= maxSyncIdAtLoad %>,
-        instanceName: '<%= instanceName %>'
+        instanceName: '<%= instanceName %>',
+        csrfToken: '<%= csrfToken %>'
     };
 </script>