From 2f8ccdab529566988b1a840b0ae98c60bc79133c Mon Sep 17 00:00:00 2001 From: Henry H Date: Tue, 6 Feb 2024 10:18:24 -0800 Subject: [PATCH] Don't allow overwriting a parent of the profile directory --- __test__/backup.test.ts | 2 +- __test__/help.test.ts | 29 +++++++++++++++++++++-------- src/Backup.ts | 8 ++++---- src/helper.ts | 20 ++++++++++++++++---- src/locales/de_DE.json | 2 +- src/locales/en_US.json | 4 ++-- 6 files changed, 45 insertions(+), 20 deletions(-) diff --git a/__test__/backup.test.ts b/__test__/backup.test.ts index c557dde..5e96ec9 100644 --- a/__test__/backup.test.ts +++ b/__test__/backup.test.ts @@ -168,7 +168,7 @@ describe("Backup", function () { }); it(`relative paths`, async () => { - const backupPath = "../"; + const backupPath = "../foo"; /* prettier-ignore */ when(spyOnsSettingsValue) .calledWith("path").mockImplementation(() => Promise.resolve(backupPath)); diff --git a/__test__/help.test.ts b/__test__/help.test.ts index ecbce06..a8966df 100644 --- a/__test__/help.test.ts +++ b/__test__/help.test.ts @@ -148,12 +148,25 @@ describe("Test helper", function () { }); test.each([ - [ "/tmp/this/is/a/test", "/tmp/this/is/a/test", true ], - [ "/tmp/test", "/tmp/test///", true ], - [ "/tmp/te", "/tmp/test", false ], - [ "a", "/a", false ], - [ "/a/b", "/b/c", false ], - ])("pathsEquivalent (%s ?= %s)", (path1, path2, expected) => { - expect(helper.pathsEquivalent(path1, path2)).toBe(expected); - }); + // Equality + ["/tmp/this/is/a/test", "/tmp/this/is/a/test", true], + ["/tmp/test", "/tmp/test///", true], + + // Subdirectories + ["/tmp", "/tmp/test", true], + ["/tmp/", "/tmp/test", true], + ["/tmp/", "/tmp/..test", true], + ["/tmp/test", "/tmp/", false], + + // Different directories + ["/tmp/", "/tmp/../test", false], + ["/tmp/te", "/tmp/test", false], + ["a", "/a", false], + ["/a/b", "/b/c", false], + ])( + "isSubdirectoryOrEqual (is %s the parent of %s?)", + (path1, path2, expected) => { + expect(helper.isSubdirectoryOrEqual(path1, path2)).toBe(expected); + } + ); }); diff --git a/src/Backup.ts b/src/Backup.ts index 6e19282..1d817e2 100644 --- a/src/Backup.ts +++ b/src/Backup.ts @@ -295,10 +295,10 @@ class Backup { await this.showError(i18n.__(errorId, invalidBackupPath)); }; - if (helper.pathsEquivalent(profileDir, this.backupBasePath)) { - await handleInvalidPath("msg.error.backupPathJoplinDir"); - } else if (helper.pathsEquivalent(os.homedir(), this.backupBasePath)) { - await handleInvalidPath("msg.error.backupPathHomeDir"); + if (helper.isSubdirectoryOrEqual(this.backupBasePath, os.homedir())) { + await handleInvalidPath("msg.error.backupPathContainsHomeDir"); + } else if (helper.isSubdirectoryOrEqual(this.backupBasePath, profileDir)) { + await handleInvalidPath("msg.error.backupPathContainsJoplinDir"); } } diff --git a/src/helper.ts b/src/helper.ts index cf2ea6f..971a49e 100644 --- a/src/helper.ts +++ b/src/helper.ts @@ -67,9 +67,21 @@ export namespace helper { return result; } - export function pathsEquivalent(path1: string, path2: string) { - // We use `resolve` and not `normalize` because `resolve` removes trailing - // slashes, while `normalize` does not. - return path.resolve(path1) === path.resolve(path2); + // Doesn't resolve simlinks + // See https://stackoverflow.com/questions/44892672/how-to-check-if-two-paths-are-the-same-in-npm + // for possible alternative implementations. + export function isSubdirectoryOrEqual(parent: string, possibleChild: string) { + // Appending path.sep to handle this case: + // parent: /a/b/test + // possibleChild: /a/b/test2 + // "/a/b/test2".startsWith("/a/b/test") -> true, but + // "/a/b/test2/".startsWith("/a/b/test/") -> false + // + // Note that .resolve removes trailing slashes. + // + const normalizedParent = path.resolve(parent) + path.sep; + const normalizedChild = path.resolve(possibleChild) + path.sep; + + return normalizedChild.startsWith(normalizedParent); } } diff --git a/src/locales/de_DE.json b/src/locales/de_DE.json index 9749df5..1f6b902 100644 --- a/src/locales/de_DE.json +++ b/src/locales/de_DE.json @@ -13,7 +13,7 @@ "Backup": "Backup Fehler für %s: %s", "fileCopy": "Fehler beim kopieren von Datei/Ordner in %s: %s", "deleteFile": "Fehler beim löschen von Datei/Ordner in %s: %s", - "backupPathJoplinDir": "Als Sicherungs Pfad wurde das Joplin profile Verzeichniss (%s) ohne Unterordner ausgewählt, dies ist nicht erlaubt!", + "backupPathContainsJoplinDir": "Als Sicherungs Pfad wurde das Joplin profile Verzeichniss (%s) ohne Unterordner ausgewählt, dies ist nicht erlaubt!", "BackupSetNotSupportedChars": "Der Name des Backup-Sets enthält nicht zulässige Zeichen ( %s )!", "passwordDoubleQuotes": "Das Passwort enthält \" (Doppelte Anführungszeichen), diese sind wegen eines Bugs nicht erlaubt. Der Passwortschutz für die Backups wurde deaktivert!" } diff --git a/src/locales/en_US.json b/src/locales/en_US.json index aadf2aa..50ad38e 100644 --- a/src/locales/en_US.json +++ b/src/locales/en_US.json @@ -13,8 +13,8 @@ "Backup": "Backup error for %s: %s", "fileCopy": "Error on file/folder copy in %s: %s", "deleteFile": "Error on file/folder delete in %s: %s", - "backupPathJoplinDir": "The backup path is the Joplin profile directory (%s) without subfolders, this is not allowed!", - "backupPathHomeDir": "The backup path is the home directory (%s). Either enable \"createSubfolders\" or choose a different backup directory.", + "backupPathContainsJoplinDir": "The backup path is or contains the Joplin profile directory (%s) without subfolders, this is not allowed!", + "backupPathContainsHomeDir": "The backup path is or contains the home directory (%s). Without enabling the subfolder setting, this is not allowed!", "BackupSetNotSupportedChars": "Backup set name does contain not allowed characters ( %s )!", "passwordDoubleQuotes": "Password contains \" (double quotes), these are not allowed because of a bug. Password protection for the backup is deactivated!" }