From c09c238e3ff5030b0a6e034c8ab261d2a4f41420 Mon Sep 17 00:00:00 2001
From: Julian Lam <julian@nodebb.org>
Date: Fri, 4 Dec 2020 13:30:28 -0500
Subject: [PATCH] fix: do not allow thumb deletion route to arbitrarily delete
 other files in uploads folder

---
 src/topics/thumbs.js  | 7 ++++---
 test/topics/thumbs.js | 7 +++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/topics/thumbs.js b/src/topics/thumbs.js
index d702b65167..b8fc9326ab 100644
--- a/src/topics/thumbs.js
+++ b/src/topics/thumbs.js
@@ -64,8 +64,9 @@ Thumbs.delete = async function (id, relativePath) {
 
 	if (associated) {
 		await db.sortedSetRemove(set, relativePath);
-	}
-	if (existsOnDisk) {
-		await file.delete(absolutePath);
+
+		if (existsOnDisk) {
+			await file.delete(absolutePath);
+		}
 	}
 };
diff --git a/test/topics/thumbs.js b/test/topics/thumbs.js
index 9be6d9eb28..05350bc3e9 100644
--- a/test/topics/thumbs.js
+++ b/test/topics/thumbs.js
@@ -127,6 +127,7 @@ describe('Topic thumbs', () => {
 
 	describe(`.delete()`, () => {
 		it('should remove a file from sorted set AND disk', async () => {
+			await topics.thumbs.associate(1, thumbPaths[0]);
 			await topics.thumbs.delete(1, thumbPaths[0]);
 
 			assert.strictEqual(await db.isSortedSetMember('topic:1:thumbs', thumbPaths[0]), false);
@@ -140,6 +141,12 @@ describe('Topic thumbs', () => {
 			assert.strictEqual(await db.isSortedSetMember(`draft:${uuid}:thumbs`, thumbPaths[1]), false);
 			assert.strictEqual(await file.exists(`${nconf.get('upload_path')}/${thumbPaths[1]}`), false);
 		});
+
+		it('should not delete the file from disk if not associated with the tid', async () => {
+			createFiles();
+			await topics.thumbs.delete(uuid, thumbPaths[0]);
+			assert.strictEqual(await file.exists(`${nconf.get('upload_path')}/${thumbPaths[0]}`), true);
+		});
 	});
 
 	describe('HTTP calls to topic thumb routes', () => {