From 12b3302687734573484cf74fdcbe15c1896a73da Mon Sep 17 00:00:00 2001
From: zadam <zadam.apps@gmail.com>
Date: Wed, 6 Jul 2022 23:09:16 +0200
Subject: [PATCH] sanitize note title && attrs just to be sure

---
 src/routes/api/clipper.js      | 8 +++++++-
 src/services/html_sanitizer.js | 2 ++
 src/services/image.js          | 3 +++
 src/services/import/zip.js     | 5 +++++
 src/services/notes.js          | 6 ++++++
 src/services/utils.js          | 2 +-
 6 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/routes/api/clipper.js b/src/routes/api/clipper.js
index b8bb158e7..ac3bb416f 100644
--- a/src/routes/api/clipper.js
+++ b/src/routes/api/clipper.js
@@ -43,7 +43,7 @@ function getClipperInboxNote() {
 }
 
 function addClipping(req) {
-    const {title, content, pageUrl, images} = req.body;
+    let {title, content, pageUrl, images} = req.body;
 
     const clipperInbox = getClipperInboxNote();
 
@@ -57,6 +57,8 @@ function addClipping(req) {
             type: 'text'
         }).note;
 
+        pageUrl = htmlSanitizer.sanitize(pageUrl);
+
         clippingNote.setLabel('clipType', 'clippings');
         clippingNote.setLabel('pageUrl', pageUrl);
         clippingNote.setLabel('iconClass', 'bx bx-globe');
@@ -89,9 +91,13 @@ function createNote(req) {
         type: 'text'
     });
 
+    clipType = htmlSanitizer.sanitize(clipType);
+
     note.setLabel('clipType', clipType);
 
     if (pageUrl) {
+        pageUrl = htmlSanitizer.sanitize(pageUrl);
+
         note.setLabel('pageUrl', pageUrl);
         note.setLabel('iconClass', 'bx bx-globe');
     }
diff --git a/src/services/html_sanitizer.js b/src/services/html_sanitizer.js
index 2a7e69698..9164ddfda 100644
--- a/src/services/html_sanitizer.js
+++ b/src/services/html_sanitizer.js
@@ -2,6 +2,8 @@ const sanitizeHtml = require('sanitize-html');
 
 // intended mainly as protection against XSS via import
 // secondarily it (partly) protects against "CSS takeover"
+// sanitize also note titles, label values etc. - there's so many usage which make it difficult to guarantee all of them
+// are properly handled
 function sanitize(dirtyHtml) {
     if (!dirtyHtml) {
         return dirtyHtml;
diff --git a/src/services/image.js b/src/services/image.js
index 19df2e874..28867ea8c 100644
--- a/src/services/image.js
+++ b/src/services/image.js
@@ -12,6 +12,7 @@ const sanitizeFilename = require('sanitize-filename');
 const noteRevisionService = require('./note_revisions');
 const isSvg = require('is-svg');
 const isAnimated = require('is-animated');
+const htmlSanitizer = require("./html_sanitizer");
 
 async function processImage(uploadBuffer, originalName, shrinkImageSwitch) {
     const compressImages = optionService.getOptionBool("compressImages");
@@ -66,6 +67,8 @@ function getImageMimeFromExtension(ext) {
 function updateImage(noteId, uploadBuffer, originalName) {
     log.info(`Updating image ${noteId}: ${originalName}`);
 
+    originalName = htmlSanitizer.sanitize(originalName);
+
     const note = becca.getNote(noteId);
 
     note.saveNoteRevision();
diff --git a/src/services/import/zip.js b/src/services/import/zip.js
index f7706b8eb..09639c3b1 100644
--- a/src/services/import/zip.js
+++ b/src/services/import/zip.js
@@ -160,6 +160,11 @@ async function importZip(taskContext, fileBuffer, importRootNote) {
                 attr.name = 'disabled:' + attr.name;
             }
 
+            if (taskContext.data.safeImport) {
+                attr.name = htmlSanitizer.sanitize(attr.name);
+                attr.value = htmlSanitizer.sanitize(attr.value);
+            }
+
             attributes.push(attr);
         }
     }
diff --git a/src/services/notes.js b/src/services/notes.js
index fe8442e1d..dad6e2c8e 100644
--- a/src/services/notes.js
+++ b/src/services/notes.js
@@ -18,6 +18,7 @@ const Branch = require('../becca/entities/branch');
 const Note = require('../becca/entities/note');
 const Attribute = require('../becca/entities/attribute');
 const dayjs = require("dayjs");
+const htmlSanitizer = require("./html_sanitizer.js");
 
 function getNewNotePosition(parentNoteId) {
     const note = becca.notes[parentNoteId];
@@ -98,6 +99,11 @@ function getNewNoteTitle(parentNote) {
         }
     }
 
+    // this isn't in theory a good place to sanitize title, but this will catch a lot of XSS attempts
+    // title is supposed to contain text only (not HTML) and be printed text only, but given the number of usages
+    // it's difficult to guarantee correct handling in all cases
+    title = htmlSanitizer.sanitize(title);
+
     return title;
 }
 
diff --git a/src/services/utils.js b/src/services/utils.js
index 85b719d01..0a3cffa86 100644
--- a/src/services/utils.js
+++ b/src/services/utils.js
@@ -241,7 +241,7 @@ function getNoteTitle(filePath, replaceUnderscoresWithSpaces, noteMeta) {
         return noteMeta.title;
     } else {
         const basename = path.basename(removeTextFileExtension(filePath));
-        if(replaceUnderscoresWithSpaces) {
+        if (replaceUnderscoresWithSpaces) {
             return basename.replace(/_/g, ' ').trim();
         }
         return basename;