From 3817cca3c619e7479137af1af998a99cd577923e Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Wed, 26 Feb 2020 16:22:57 +0100 Subject: [PATCH] Throw on unarmored messages with garbage data appended --- src/openpgp.js | 15 +++++++++++++-- src/packet/packet.js | 10 +++------- test/general/openpgp.js | 9 +++++++++ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/openpgp.js b/src/openpgp.js index 63961c99..a0a9a993 100644 --- a/src/openpgp.js +++ b/src/openpgp.js @@ -369,7 +369,7 @@ export function decrypt({ message, privateKeys, passwords, sessionKeys, publicKe result.signatures = signature ? await decrypted.verifyDetached(signature, publicKeys, date, streaming) : await decrypted.verify(publicKeys, date, streaming); result.data = format === 'binary' ? decrypted.getLiteralData() : decrypted.getText(); result.filename = decrypted.getFilename(); - if (streaming) linkStreams(result, message); + linkStreams(result, message); result.data = await convertStream(result.data, streaming, format); if (!streaming) await prepareSignatures(result.signatures); return result; @@ -635,13 +635,24 @@ async function convertStream(data, streaming, encoding = 'utf8') { /** * Link result.data to the message stream for cancellation. + * Also, forward errors in the message to result.data. * @param {Object} result the data to convert * @param {Message} message message object * @returns {Object} */ function linkStreams(result, message) { result.data = stream.transformPair(message.packets.stream, async (readable, writable) => { - await stream.pipe(result.data, writable); + await stream.pipe(result.data, writable, { + preventClose: true + }); + const writer = stream.getWriter(writable); + try { + // Forward errors in the message stream to result.data. + await stream.readToEnd(readable, _ => _); + await writer.close(); + } catch (e) { + await writer.abort(e); + } }); } diff --git a/src/packet/packet.js b/src/packet/packet.js index af399620..49b61b2f 100644 --- a/src/packet/packet.js +++ b/src/packet/packet.js @@ -249,13 +249,9 @@ export default { // entire remainder of the stream, in order to forward errors in the // remainder of the stream to the packet data. (Note that this means we // read/peek at all signature packets before closing the literal data - // packet, for example.) This forwards armor checksum errors to the - // encrypted data stream, for example, so that they don't get lost / - // forgotten on encryptedMessage.packets.stream, which we never look at. - // - // Note that subsequent packet parsing errors could still end up there if - // `config.tolerant` is set to false, or on malformed messages with - // multiple data packets, but usually it shouldn't happen. + // packet, for example.) This forwards MDC errors to the literal data + // stream, for example, so that they don't get lost / forgotten on + // decryptedMessage.packets.stream, which we never look at. // // An example of what we do when stream-parsing a message containing // [ one-pass signature packet, literal data packet, signature packet ]: diff --git a/test/general/openpgp.js b/test/general/openpgp.js index 3701a25c..3a316282 100644 --- a/test/general/openpgp.js +++ b/test/general/openpgp.js @@ -1700,6 +1700,15 @@ describe('OpenPGP.js public api tests', function() { })); } }); + + it('should fail to decrypt unarmored message with garbage data appended', async function() { + const { key } = await openpgp.generateKey({ userIds: {} }); + const message = await openpgp.encrypt({ message: openpgp.message.fromText('test'), publicKeys: key, privateKeys: key, armor: false }); + const encrypted = openpgp.util.concat([message, new Uint8Array([11])]); + await expect( + openpgp.decrypt({ message: await openpgp.message.read(encrypted), privateKeys: key, publicKeys: key }) + ).to.be.rejectedWith('Error during parsing. This message / key probably does not conform to a valid OpenPGP format.'); + }); }); describe('ELG / DSA encrypt, decrypt, sign, verify', function() {