From 4fc686bbbc023d2ffd2ba1e46b8aaf0782a48fc8 Mon Sep 17 00:00:00 2001
From: zadam <zadam.apps@gmail.com>
Date: Tue, 5 Jul 2022 22:40:41 +0200
Subject: [PATCH 1/2] sanitize note title

---
 src/public/app/dialogs/options/appearance.js          | 2 +-
 src/public/app/services/note_content_renderer.js      | 6 ++++--
 src/public/app/services/note_list_renderer.js         | 2 +-
 src/public/app/services/tab_manager.js                | 2 +-
 src/public/app/services/toast.js                      | 9 +++++----
 src/public/app/widgets/ribbon_widgets/edited_notes.js | 4 +++-
 src/public/app/widgets/type_widgets/editable_text.js  | 3 ++-
 src/public/app/widgets/type_widgets/empty.js          | 2 +-
 src/services/notes.js                                 | 4 +++-
 9 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/public/app/dialogs/options/appearance.js b/src/public/app/dialogs/options/appearance.js
index 3fc914498..db7e0f593 100644
--- a/src/public/app/dialogs/options/appearance.js
+++ b/src/public/app/dialogs/options/appearance.js
@@ -297,7 +297,7 @@ export default class ApperanceOptions {
             this.$themeSelect.append($("<option>")
                 .attr("value", theme.val)
                 .attr("data-note-id", theme.noteId)
-                .html(theme.title));
+                .text(theme.title));
         }
 
         this.$themeSelect.val(options.theme);
diff --git a/src/public/app/services/note_content_renderer.js b/src/public/app/services/note_content_renderer.js
index 771f860b7..61966b0b4 100644
--- a/src/public/app/services/note_content_renderer.js
+++ b/src/public/app/services/note_content_renderer.js
@@ -61,9 +61,11 @@ async function getRenderedContent(note, options = {}) {
         $renderedContent.append($("<pre>").text(trim(fullNote.content, options.trim)));
     }
     else if (type === 'image') {
+        const sanitizedTitle = note.title.replace(/[^a-z0-9-.]/gi, "");
+
         $renderedContent.append(
             $("<img>")
-                .attr("src", `api/images/${note.noteId}/${note.title}`)
+                .attr("src", `api/images/${note.noteId}/${sanitizedTitle}`)
                 .css("max-width", "100%")
         );
     }
@@ -144,7 +146,7 @@ async function getRenderedContent(note, options = {}) {
     else if (type === 'canvas') {
         // make sure surrounding container has size of what is visible. Then image is shrinked to its boundaries
         $renderedContent.css({height: "100%", width:"100%"});
-        
+
         const noteComplement = await froca.getNoteComplement(note.noteId);
         const content = noteComplement.content || "";
 
diff --git a/src/public/app/services/note_list_renderer.js b/src/public/app/services/note_list_renderer.js
index 705a0d31f..1006374a5 100644
--- a/src/public/app/services/note_list_renderer.js
+++ b/src/public/app/services/note_list_renderer.js
@@ -266,7 +266,7 @@ class NoteListRenderer {
                     .append($expander)
                     .append($('<span class="note-icon">').addClass(note.getIcon()))
                     .append(this.viewType === 'grid'
-                        ? note.title
+                        ? $("<span>").text(note.title)
                         : await linkService.createNoteLink(notePath, {showTooltip: false, showNotePath: this.showNotePath})
                     )
                     .append($renderedAttributes)
diff --git a/src/public/app/services/tab_manager.js b/src/public/app/services/tab_manager.js
index 59318d6e3..e4d8aec96 100644
--- a/src/public/app/services/tab_manager.js
+++ b/src/public/app/services/tab_manager.js
@@ -482,7 +482,7 @@ export default class TabManager extends Component {
 
     updateDocumentTitle(activeNoteContext) {
         const titleFragments = [
-            // it helps navigating in history if note title is included in the title
+            // it helps to navigate in history if note title is included in the title
             activeNoteContext.note?.title,
             "Trilium Notes"
         ].filter(Boolean);
diff --git a/src/public/app/services/toast.js b/src/public/app/services/toast.js
index d51cad528..ff2871044 100644
--- a/src/public/app/services/toast.js
+++ b/src/public/app/services/toast.js
@@ -4,16 +4,17 @@ import utils from "./utils.js";
 function toast(options) {
     const $toast = $(`<div class="toast" role="alert" aria-live="assertive" aria-atomic="true">
     <div class="toast-header">
-        <strong class="mr-auto"><span class="bx bx-${options.icon}"></span> ${options.title}</strong>
+        <strong class="mr-auto"><span class="bx bx-${options.icon}"></span> <span class="toast-title"></span></strong>
         <button type="button" class="ml-2 mb-1 close" data-dismiss="toast" aria-label="Close">
             <span aria-hidden="true">&times;</span>
         </button>
     </div>
-    <div class="toast-body">
-        ${options.message}
-    </div>
+    <div class="toast-body"></div>
 </div>`);
 
+    $toast.find('.toast-title').text(options.title);
+    $toast.find('.toast-body').text(options.message);
+
     if (options.id) {
         $toast.attr("id", "toast-" + options.id);
     }
diff --git a/src/public/app/widgets/ribbon_widgets/edited_notes.js b/src/public/app/widgets/ribbon_widgets/edited_notes.js
index 629fbeeea..2b86e1d95 100644
--- a/src/public/app/widgets/ribbon_widgets/edited_notes.js
+++ b/src/public/app/widgets/ribbon_widgets/edited_notes.js
@@ -77,7 +77,9 @@ export default class EditedNotesWidget extends CollapsibleWidget {
                 );
             }
             else {
-                $item.append(editedNote.notePath ? await linkService.createNoteLink(editedNote.notePath.join("/"), {showNotePath: true}) : editedNote.title);
+                $item.append(editedNote.notePath
+                    ? await linkService.createNoteLink(editedNote.notePath.join("/"), {showNotePath: true})
+                    : $("<span>").text(editedNote.title));
             }
 
             if (i < editedNotes.length - 1) {
diff --git a/src/public/app/widgets/type_widgets/editable_text.js b/src/public/app/widgets/type_widgets/editable_text.js
index d86beb5d3..68aa072d0 100644
--- a/src/public/app/widgets/type_widgets/editable_text.js
+++ b/src/public/app/widgets/type_widgets/editable_text.js
@@ -296,7 +296,8 @@ export default class EditableTextTypeWidget extends AbstractTextTypeWidget {
         const note = await froca.getNote(noteId);
 
         this.textEditor.model.change( writer => {
-            const src = `api/images/${note.noteId}/${note.title}`;
+            const sanitizedTitle = note.title.replace(/[^a-z0-9-.]/gi, "");
+            const src = `api/images/${note.noteId}/${sanitizedTitle}`;
 
             const imageElement = writer.createElement( 'image',  { 'src': src } );
 
diff --git a/src/public/app/widgets/type_widgets/empty.js b/src/public/app/widgets/type_widgets/empty.js
index 6a718084e..2e3fd97f9 100644
--- a/src/public/app/widgets/type_widgets/empty.js
+++ b/src/public/app/widgets/type_widgets/empty.js
@@ -79,7 +79,7 @@ export default class EmptyTypeWidget extends TypeWidget {
             this.$workspaceNotes.append(
                 $('<div class="workspace-note">')
                     .append($("<div>").addClass(workspaceNote.getIcon() + " workspace-icon"))
-                    .append($("<div>").append(workspaceNote.title))
+                    .append($("<div>").text(workspaceNote.title))
                     .attr("title", "Enter workspace " + workspaceNote.title)
                     .on('click', () => this.triggerCommand('hoistNote', {noteId: workspaceNote.noteId}))
             );
diff --git a/src/services/notes.js b/src/services/notes.js
index 15526e9b6..fe8442e1d 100644
--- a/src/services/notes.js
+++ b/src/services/notes.js
@@ -341,8 +341,10 @@ function downloadImages(noteId, content) {
             const imageService = require('../services/image');
             const {note} = imageService.saveImage(noteId, imageBuffer, "inline image", true, true);
 
+            const sanitizedTitle = note.title.replace(/[^a-z0-9-.]/gi, "");
+
             content = content.substr(0, imageMatch.index)
-                + `<img src="api/images/${note.noteId}/${note.title}"`
+                + `<img src="api/images/${note.noteId}/${sanitizedTitle}"`
                 + content.substr(imageMatch.index + imageMatch[0].length);
         }
         else if (!url.includes('api/images/')

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 2/2] 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;