From cc63ffa6bbdb0ead3cd03ec8294dc52cfc13e3e8 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 13 Mar 2025 09:57:50 +0100 Subject: [PATCH 1/3] Reduce duplication when checking the userPassword, in `CipherTransformFactory.prototype.#prepareKeyData` Currently we duplicate the exact same code in both the `if`- and `else`-branches, which seems unnecessary, and we can also replace the manual loop. --- src/core/crypto.js | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/core/crypto.js b/src/core/crypto.js index c5ebdefbc..e11f579e0 100644 --- a/src/core/crypto.js +++ b/src/core/crypto.js @@ -916,23 +916,15 @@ class CipherTransformFactory { cipher = new ARCFourCipher(derivedKey); checkData = cipher.encryptBlock(checkData); } - for (j = 0, n = checkData.length; j < n; ++j) { - if (userPassword[j] !== checkData[j]) { - return null; - } - } } else { cipher = new ARCFourCipher(encryptionKey); checkData = cipher.encryptBlock( CipherTransformFactory._defaultPasswordBytes ); - for (j = 0, n = checkData.length; j < n; ++j) { - if (userPassword[j] !== checkData[j]) { - return null; - } - } } - return encryptionKey; + return checkData.every((data, k) => userPassword[k] === data) + ? encryptionKey + : null; } #decodeUserPassword(password, ownerPassword, revision, keyLength) { From 0e15f709c4a9b8804a0d4cc99b8fb9d1d5e9fc17 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 13 Mar 2025 10:17:15 +0100 Subject: [PATCH 2/3] Remove unnecessary `else if` when checking the encryptionKey in the `CipherTransformFactory` constructor This can be simplified a tiny bit since we already throw `PasswordException` when no password is provided. --- src/core/crypto.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/crypto.js b/src/core/crypto.js index e11f579e0..a6cc5e8bf 100644 --- a/src/core/crypto.js +++ b/src/core/crypto.js @@ -1125,12 +1125,13 @@ class CipherTransformFactory { perms ); } - if (!encryptionKey && !password) { - throw new PasswordException( - "No password given", - PasswordResponses.NEED_PASSWORD - ); - } else if (!encryptionKey && password) { + if (!encryptionKey) { + if (!password) { + throw new PasswordException( + "No password given", + PasswordResponses.NEED_PASSWORD + ); + } // Attempting use the password as an owner password const decodedPassword = this.#decodeUserPassword( passwordBytes, From ef01ceda1bb1ab0bbc952c8dc03c64d78e7651b5 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 13 Mar 2025 11:49:27 +0100 Subject: [PATCH 3/3] Change a couple of "password" ref-tests to "eq" tests Currrently these are just "load" tests, and by also testing rendering we get slightly better test-coverage for the `src/core/crypto.js` file. --- test/test_manifest.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_manifest.json b/test/test_manifest.json index f7ea4f2c8..f5e178215 100644 --- a/test/test_manifest.json +++ b/test/test_manifest.json @@ -992,7 +992,7 @@ "password": "ELXRTQWS", "md5": "db2fedbd36d6fa27d4e52f9bd2d96b8c", "rounds": 1, - "type": "load" + "type": "eq" }, { "id": "issue5334", @@ -5961,7 +5961,7 @@ "md5": "b58adce5dbb08936ddb0d904f0da8716", "rounds": 1, "link": false, - "type": "load", + "type": "eq", "password": "abc" }, { @@ -5970,7 +5970,7 @@ "md5": "73a8091d0ab2a47af5ca45047f04da99", "rounds": 1, "link": false, - "type": "load", + "type": "eq", "password": "\u00E6\u00F8\u00E5", "about": "The password (æøå) is UTF8 encoded." },