From d1c165e1583262f9e17f9994f198f1527887498f Mon Sep 17 00:00:00 2001 From: Vitaliy Pavlenko Date: Tue, 27 Dec 2022 23:10:27 +0200 Subject: [PATCH] cipher: add cipher update/final methods encoding validation Adds encoding validation to update and final cipher methods. https://github.com/nodejs/node/issues/45189 --- README.md | 2 + doc/api/errors.md | 6 +++ lib/internal/crypto/cipher.js | 30 ++++++++++++-- lib/internal/errors.js | 1 + lib/internal/validators.js | 2 + .../test-crypto-encoding-validation-error.js | 41 +++++++++++++++++++ 6 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-crypto-encoding-validation-error.js diff --git a/README.md b/README.md index 01fea35069a42e..a086fc7fcf6636 100644 --- a/README.md +++ b/README.md @@ -446,6 +446,8 @@ For information about the governance of the Node.js project, see **Rich Trott** <> (he/him) * [vdeturckheim](https://github.com/vdeturckheim) - **Vladimir de Turckheim** <> (he/him) +* [vitpavlenko](https://github.com/vitpavlenko) - + **Vitaliy Pavlenko** <> (he/him) * [VoltrexKeyva](https://github.com/VoltrexKeyva) - **Mohammed Keyvanzadeh** <> (he/him) * [watilde](https://github.com/watilde) - diff --git a/doc/api/errors.md b/doc/api/errors.md index b277a53e66cd6d..fe6a417ae6cc61 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1112,6 +1112,12 @@ release binaries but can happen with custom builds, including distro builds. A signing `key` was not provided to the [`sign.sign()`][] method. + + +### `ERR_CRYPTO_UNKNOWN_ENCODING` + +Invalid encoding was provided to the update/finalise cipher methods. + ### `ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH` diff --git a/lib/internal/crypto/cipher.js b/lib/internal/crypto/cipher.js index d85606ba52b5ac..1ffd4c10e27f4e 100644 --- a/lib/internal/crypto/cipher.js +++ b/lib/internal/crypto/cipher.js @@ -27,6 +27,7 @@ const { ERR_CRYPTO_INVALID_STATE, ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, + ERR_CRYPTO_UNKNOWN_ENCODING, } } = require('internal/errors'); @@ -90,10 +91,31 @@ const privateEncrypt = rsaFunctionFor(_privateEncrypt, RSA_PKCS1_PADDING, const privateDecrypt = rsaFunctionFor(_privateDecrypt, RSA_PKCS1_OAEP_PADDING, 'private'); +const validateNormalizedEncoding = (encoding, originalEncodingName) => { + if (!encoding) { + throw new ERR_CRYPTO_UNKNOWN_ENCODING(originalEncodingName); + } + + return encoding; +}; + +const normalizeAndValidateEncoding = (encoding) => { + return validateNormalizedEncoding(normalizeEncoding(encoding), encoding); +}; + +const validateInputEncoding = (encoding) => { + if (encoding === 'buffer') { + return; + } + + return normalizeAndValidateEncoding(encoding); +}; + function getDecoder(decoder, encoding) { - encoding = normalizeEncoding(encoding); - decoder = decoder || new StringDecoder(encoding); - assert(decoder.encoding === encoding, 'Cannot change encoding'); + const normilizedEncoding = normalizeAndValidateEncoding(encoding); + + decoder = decoder || new StringDecoder(normilizedEncoding); + assert(decoder.encoding === normilizedEncoding, 'Cannot change encoding'); return decoder; } @@ -177,6 +199,8 @@ Cipher.prototype.update = function update(data, inputEncoding, outputEncoding) { 'data', ['string', 'Buffer', 'TypedArray', 'DataView'], data); } + validateInputEncoding(inputEncoding); + const ret = this[kHandle].update(data, inputEncoding); if (outputEncoding && outputEncoding !== 'buffer') { diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 5820747e0d7fcf..07acdb306874a6 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1015,6 +1015,7 @@ E('ERR_CRYPTO_SCRYPT_INVALID_PARAMETER', 'Invalid scrypt parameter', Error); E('ERR_CRYPTO_SCRYPT_NOT_SUPPORTED', 'Scrypt algorithm not supported', Error); // Switch to TypeError. The current implementation does not seem right. E('ERR_CRYPTO_SIGN_KEY_REQUIRED', 'No key provided to sign', Error); +E('ERR_CRYPTO_UNKNOWN_ENCODING', 'Unknown encoding %s', TypeError); E('ERR_DEBUGGER_ERROR', '%s', Error); E('ERR_DEBUGGER_STARTUP_ERROR', '%s', Error); E('ERR_DIR_CLOSED', 'Directory handle was closed', Error); diff --git a/lib/internal/validators.js b/lib/internal/validators.js index a971e6574621c5..07e30d2569feca 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -371,6 +371,8 @@ function validateEncoding(data, encoding) { throw new ERR_INVALID_ARG_VALUE('encoding', encoding, `is invalid for data of length ${length}`); } + + return normalizedEncoding; } /** diff --git a/test/parallel/test-crypto-encoding-validation-error.js b/test/parallel/test-crypto-encoding-validation-error.js new file mode 100644 index 00000000000000..93ffca48a5f121 --- /dev/null +++ b/test/parallel/test-crypto-encoding-validation-error.js @@ -0,0 +1,41 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +// This test checks if error is thrown in case of wrong encoding provided into cipher. + +const assert = require('assert'); +const { createCipheriv, randomBytes } = require('crypto'); + +const createCipher = () => { + return createCipheriv('aes-256-cbc', randomBytes(32), randomBytes(16)); +}; + +{ + const cipher = createCipher(); + + assert.throws( + () => cipher.update('test', 'bad1', 'hex'), + { message: /^Unknown encoding bad1$/ } + ); +} + +{ + const cipher = createCipher(); + cipher.update('test', 'utf-8', 'utf-8'); + + assert.throws( + () => cipher.final('bad2'), + { message: /^Unknown encoding bad2$/ } + ); +} + +{ + const cipher = createCipher(); + + assert.throws( + () => cipher.update('test', 'utf-8', 'bad3'), + { message: /^Unknown encoding bad3$/ } + ); +}