Skip to content

Commit

Permalink
Don't allow overwriting a parent of the profile directory
Browse files Browse the repository at this point in the history
  • Loading branch information
personalizedrefrigerator committed Feb 6, 2024
1 parent 42ac15d commit 2f8ccda
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 20 deletions.
2 changes: 1 addition & 1 deletion __test__/backup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
29 changes: 21 additions & 8 deletions __test__/help.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
);
});
8 changes: 4 additions & 4 deletions src/Backup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}

Expand Down
20 changes: 16 additions & 4 deletions src/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
2 changes: 1 addition & 1 deletion src/locales/de_DE.json
Original file line number Diff line number Diff line change
Expand Up @@ -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!"
}
Expand Down
4 changes: 2 additions & 2 deletions src/locales/en_US.json
Original file line number Diff line number Diff line change
Expand Up @@ -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!"
}
Expand Down

0 comments on commit 2f8ccda

Please sign in to comment.