Skip to content

Commit 127f79a

Browse files
maruthangnodejs-github-bot
authored andcommitted
crypto: validate inputEncoding in Cipher/Decipher update
Cipher.update(string, badEncoding, ...) and Decipher.update with the same shape silently produced incorrect output: the binding skipped the unrecognized encoding and fell back to a default, giving the user wrong ciphertext or plaintext with no signal. Sub-cases 1 and 2 from issue #45189 (bad output encoding to update/final) were addressed in PR #45990. This commit completes the fix for sub-case 3 (bad input encoding) per panva's comment deferring it to a follow-up PR for CITGM testing. When `data` is a string and `inputEncoding` is non-null but does not normalize to a known encoding, throw ERR_UNKNOWN_ENCODING. Buffer / TypedArray / DataView data paths are unaffected (the binding ignores `inputEncoding` for non-string data anyway). Fixes: #45189 Refs: #45990 Signed-off-by: Maruthan G <maruthang4@gmail.com> PR-URL: #66247 Reviewed-By: Xuguang Mei <meixuguang@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent f71d644 commit 127f79a

2 files changed

Lines changed: 53 additions & 0 deletions

File tree

‎lib/internal/crypto/cipher.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,10 @@ function _flush(callback) {
185185
function update(data, inputEncoding, outputEncoding) {
186186
if (typeof data === 'string') {
187187
validateEncoding(data, inputEncoding);
188+
if (inputEncoding != null &&
189+
normalizeEncoding(inputEncoding) === undefined) {
190+
throw new ERR_UNKNOWN_ENCODING(inputEncoding);
191+
}
188192
} else if (!isArrayBufferView(data)) {
189193
throw new ERR_INVALID_ARG_TYPE(
190194
'data', ['string', 'Buffer', 'TypedArray', 'DataView'], data);

‎test/parallel/test-crypto-encoding-validation-error.js‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,52 @@ const encodingChangeError = {
5656
{ message: /^Unknown encoding: bad3$/, code: 'ERR_UNKNOWN_ENCODING' }
5757
);
5858
}
59+
60+
// Regression tests for https://github.com/nodejs/node/issues/45189:
61+
// Unknown input encodings used to be silently accepted by Cipher/Decipher
62+
// `update`, producing incorrect (and silently non-deterministic) output.
63+
// They must now reject with ERR_UNKNOWN_ENCODING.
64+
65+
{
66+
const cipher = createCipher();
67+
68+
assert.throws(
69+
() => cipher.update('test', 'bad', 'hex'),
70+
{ message: /^Unknown encoding: bad$/, code: 'ERR_UNKNOWN_ENCODING' }
71+
);
72+
}
73+
74+
{
75+
const { createDecipheriv } = require('crypto');
76+
const decipher = createDecipheriv(
77+
'aes-256-cbc', randomBytes(32), randomBytes(16));
78+
79+
assert.throws(
80+
() => decipher.update('test', 'bad', 'hex'),
81+
{ message: /^Unknown encoding: bad$/, code: 'ERR_UNKNOWN_ENCODING' }
82+
);
83+
}
84+
85+
// A buffer-like data argument should not trigger encoding validation,
86+
// because the input encoding is ignored when data is not a string.
87+
{
88+
const cipher = createCipher();
89+
// Should not throw.
90+
cipher.update(Buffer.from('test'), 'bad-but-ignored', 'hex');
91+
}
92+
93+
// Valid input encodings must continue to work.
94+
{
95+
const cipher = createCipher();
96+
let result = cipher.update('test', 'utf-8', 'hex');
97+
result += cipher.final('hex');
98+
assert.strictEqual(typeof result, 'string');
99+
}
100+
101+
// Omitting the input encoding (undefined / null) is allowed; the
102+
// underlying binding falls back to its default behavior.
103+
{
104+
const cipher = createCipher();
105+
// Should not throw.
106+
cipher.update(Buffer.from('test'));
107+
}

0 commit comments

Comments
 (0)