From 7c59f1295065005f43697b5956205b96e997a3fc Mon Sep 17 00:00:00 2001 From: lieser Date: Sat, 3 Jul 2021 21:23:27 +0200 Subject: [PATCH] show proper error message on ill-formed from (#238) --- CHANGELOG.txt | 2 ++ _locales/de/messages.json | 3 ++ _locales/en_US/messages.json | 3 ++ modules/authVerifier.mjs.js | 19 +++++++++-- test/data/ill_formed-from.eml | 14 ++++++++ test/unittest/authVerifierSpec.mjs.js | 46 +++++++++++++++++++-------- 6 files changed, 72 insertions(+), 15 deletions(-) create mode 100644 test/data/ill_formed-from.eml diff --git a/CHANGELOG.txt b/CHANGELOG.txt index d9ebb27c..368df46d 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,5 @@ + - show proper error message on ill-formed from (#238) + 4.0.0 [2021-04-18] ------------------ - now requires at least Thunderbird 78 diff --git a/_locales/de/messages.json b/_locales/de/messages.json index dc8c14ca..c8d06b69 100644 --- a/_locales/de/messages.json +++ b/_locales/de/messages.json @@ -34,6 +34,9 @@ "DKIM_INTERNALERROR_INCORRECT_EMAIL_FORMAT": { "message": "E-Mail ist nicht richtig formatiert" }, + "DKIM_INTERNALERROR_INCORRECT_FROM": { + "message": "Absenderadresse ist falsch formatiert" + }, // DKIM_INTERNALERROR - DNS "DKIM_DNSERROR_UNKNOWN": { "message": "Fehler im DNS-Resolver" diff --git a/_locales/en_US/messages.json b/_locales/en_US/messages.json index 6c8a81f8..9d52a08a 100644 --- a/_locales/en_US/messages.json +++ b/_locales/en_US/messages.json @@ -34,6 +34,9 @@ "DKIM_INTERNALERROR_INCORRECT_EMAIL_FORMAT": { "message": "E-mail is not correctly formatted" }, + "DKIM_INTERNALERROR_INCORRECT_FROM": { + "message": "From address is ill-formed" + }, // DKIM_INTERNALERROR - DNS "DKIM_DNSERROR_UNKNOWN": { "message": "Error in the DNS resolver" diff --git a/modules/authVerifier.mjs.js b/modules/authVerifier.mjs.js index 335cb24c..b70233f7 100644 --- a/modules/authVerifier.mjs.js +++ b/modules/authVerifier.mjs.js @@ -124,10 +124,25 @@ export default class AuthVerifier { if (!fromHeader) { throw new Error("message does not contain a from header"); } + let from; + try { + from = MsgParser.parseFromHeader(fromHeader[0]); + } catch (error) { + log.error("Parsing of from header failed", error); + return Promise.resolve({ + version: "2.1", + dkim: [{ + version: "2.0", + result: "PERMFAIL", + res_num: 30, + result_str: browser.i18n.getMessage("DKIM_INTERNALERROR_INCORRECT_FROM"), + }], + }); + } const msg = { headerFields: msgParsed.headers, bodyPlain: msgParsed.body, - from: MsgParser.parseFromHeader(fromHeader[0]), + from: from, }; const listIdHeader = msgParsed.headers.get("list-id"); let listId = ""; @@ -774,7 +789,7 @@ function SavedAuthResult_to_AuthResult(savedAuthResult) { } /** - * Convert AuthResultV2 to dkimSigResultV2 + * Convert AuthResultDKIMV2 to dkimSigResultV2 * * @param {AuthResultDKIMV2} authResultDKIM * @return {VerifierModule.dkimSigResultV2} dkimSigResultV2 diff --git a/test/data/ill_formed-from.eml b/test/data/ill_formed-from.eml new file mode 100644 index 00000000..ff98c9cd --- /dev/null +++ b/test/data/ill_formed-from.eml @@ -0,0 +1,14 @@ +Received: from client1.football.example.com [192.0.2.1] + by submitserver.example.com with SUBMISSION; + Fri, 11 Jul 2003 21:01:54 -0700 (PDT) +From: Joe SixPack +To: Suzie Q +Subject: Is dinner ready? (ill-formed from) +Date: Fri, 11 Jul 2003 21:00:37 -0700 (PDT) +Message-ID: <20030712040037.46341.5F8J-ill_formed-from@football.example.com> + +Hi. + +We lost the game. Are you hungry yet? + +Joe. diff --git a/test/unittest/authVerifierSpec.mjs.js b/test/unittest/authVerifierSpec.mjs.js index 57b6bade..cee52d1e 100644 --- a/test/unittest/authVerifierSpec.mjs.js +++ b/test/unittest/authVerifierSpec.mjs.js @@ -45,6 +45,20 @@ function createFakeMessageHeader() { }; } +/** + * @param {Map} headers + * @param {string} name + * @returns {string[]} + */ +function extractHeaderValue(headers, name) { + const completeHeaders = headers.get(name); + if (completeHeaders === undefined) { + return []; + } + return completeHeaders.map(header => + header.substr(name.length + ": ".length).slice(0, -"\r\n".length)); +} + /** * @param {string} file - path to file relative to test data directory * @returns {Promise} @@ -53,12 +67,12 @@ async function createMessageHeader(file) { const fakeMessageHeader = createFakeMessageHeader(); const msgPlain = await readTestFile(file); const msgParsed = MsgParser.parseMsg(msgPlain); - fakeMessageHeader.author = (msgParsed.headers.get("from") ?? [])[0]; - fakeMessageHeader.recipients = msgParsed.headers.get("to") ?? []; - fakeMessageHeader.subject = (msgParsed.headers.get("subject") ?? [])[0]; + fakeMessageHeader.author = extractHeaderValue(msgParsed.headers, "from")[0]; + fakeMessageHeader.recipients = extractHeaderValue(msgParsed.headers, "to"); + fakeMessageHeader.subject = extractHeaderValue(msgParsed.headers, "subject")[0]; browser.messages = { getRaw: sinon.fake.resolves(msgPlain), - getFull: sinon.fake.throws("no fake for browser.messages.messages"), + getFull: sinon.fake.throws("no fake for browser.messages.getFull"), }; return fakeMessageHeader; } @@ -73,6 +87,9 @@ describe("AuthVerifier [unittest]", function () { this.skip(); } await prefs.init(); + }); + + beforeEach(async function () { await prefs.clear(); }); @@ -80,7 +97,7 @@ describe("AuthVerifier [unittest]", function () { /** @type {import("../../modules/dkim/verifier.mjs.js").dkimResultV1|import("../../modules/authVerifier.mjs.js").AuthResultV2|import("../../modules/authVerifier.mjs.js").SavedAuthResultV3} */ let storedData; - before(async function () { + beforeEach(async function () { await prefs.setValue("saveResult", true); browser.storageMessage = { @@ -192,11 +209,8 @@ describe("AuthVerifier [unittest]", function () { expect(res.dmarc && res.dmarc[0].result).to.be.equal("pass"); }); }); - describe("sign rules", function () { - beforeEach(async function () { - await prefs.clear(); - }); + describe("sign rules", function () { it("unsigned PayPal message", async function () { const fakePayPalMessage = await createMessageHeader("fakePayPal.eml"); let res = await authVerifier.verify(fakePayPalMessage); @@ -239,11 +253,8 @@ describe("AuthVerifier [unittest]", function () { expect(res.dkim[0].result_str).to.be.equal("Invalid (Should be signed by paypal.com)"); }); }); - describe("ARH header", function () { - beforeEach(async function () { - await prefs.clear(); - }); + describe("ARH header", function () { it("spf and dkim result", async function () { const message = await createMessageHeader("rfc6376-A.2-arh-valid.eml"); let res = await authVerifier.verify(message); @@ -327,4 +338,13 @@ describe("AuthVerifier [unittest]", function () { }); }); }); + + describe("invalid messages", function () { + it("ill-formed from shows proper error message", async function () { + const message = await createMessageHeader("ill_formed-from.eml"); + const res = await authVerifier.verify(message); + expect(res.dkim[0].result).to.be.equal("PERMFAIL"); + expect(res.dkim[0].result_str).to.be.equal("From address is ill-formed"); + }); + }); });