crypto.Decipher.update errors when inputEncoding is provided and data is a Buffer

[benji2212]

• Version: 13.8.0
• Platform: any as far as I can tell - discovered on Windows 10 Enterprise 64bit
• Subsystem: crypto

What steps will reproduce the bug?

Creating a piece of content to encrypt with an odd length, encrypting it using cipher then calling decipher.update with a buffer containing the encrypted value and 'hex' as the inputEncoding will reproduce the error.

const crypto = require('crypto');

const key = '76814765872355644465872785667516';
const iv = '594835440127';
const content = 'the content to be encrypted seems to need to be an odd length to reproduce the issue correctly.';

const cipher = crypto.createCipheriv('aes-256-gcm', key, iv);
let encrypted = cipher.update(content, 'utf8', 'hex');
encrypted += cipher.final('hex');
const authTag = cipher.getAuthTag().toString('hex');

const encryptedBuffer = Buffer.from(encrypted, 'hex');
const authTagBuffer = Buffer.from(authTag, 'hex');
const decipher = crypto.createDecipheriv('aes-256-gcm', key, iv);
decipher.setAuthTag(authTagBuffer);
let decrypted = decipher.update(encryptedBuffer, 'hex', 'utf8');
decrypted += decipher.final('utf8');

console.log(decrypted);

How often does it reproduce? Is there a required condition?

Should be consistently reproducible with the above code

What is the expected behavior?

The docs for Decipher.update state that inputEncoding should be ignored when the data argument is a Buffer:
Updates the decipher with data. If the inputEncoding argument is given, the data argument is a string using the specified encoding. If the inputEncoding argument is not given, data must be a Buffer. If data is a Buffer then inputEncoding is ignored.

The input encoding is instead being validated and failing validation. If the inputEncoding is provided as null in the example above, decryption happens correctly.

The above code snippet should log out 'the content to be encrypted seems to need to be an odd length to reproduce the issue correctly.'

What do you see instead?

internal/validators.js:153
throw new ERR_INVALID_ARG_VALUE('encoding', encoding,
^

TypeError [ERR_INVALID_ARG_VALUE]: The argument 'encoding' is invalid for data of length 95. Received 'hex'
at validateEncoding (internal/validators.js:153:11)
at Decipheriv.update (internal/crypto/cipher.js:159:3)
at Object. (C:\Users\n0139509\IdeaProjects\azure-node-auth\example.js:19:26)
at Module._compile (internal/modules/cjs/loader.js:1151:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1171:10)
at Module.load (internal/modules/cjs/loader.js:1000:32)
at Function.Module._load (internal/modules/cjs/loader.js:899:14)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
at internal/main/run_main_module.js:17:47 {
code: 'ERR_INVALID_ARG_VALUE'
}

Additional information

This works properly in all versions of node I've tried prior to 13 (tried 10 and 12). It seems to be caused by the addition of the validateEncoding call to the Cipher.prototype.update function in internal/crypto/cipher.js without any check for whether the data provided is a Buffer or not, but I'm not sure if the docs were meant to change, or if the possibility of a Buffer being provided was overlooked when that call was added.

[bnoordhuis]

This line:

let decrypted = decipher.update(encryptedBuffer, 'hex', 'utf8');

Should read:

let decrypted = decipher.update(encryptedBuffer, 'buffer', 'utf8');

The validation rejects the input when it's not a string and encoding='hex'. That makes perfect sense, IMO, but it's not the documented behavior.

It's inconsistent: sign.update() works according to the docs, cipher/decipher/hash/hmac all don't.

I'm inclined to update the docs and change sign.update() because that's more likely to catch bugs than the other way around.

[bnoordhuis]

Oh wait, it's worse than that: cipher/decipher/etc. selectively reject the buffer based on its length, even though inputEncoding='hex' is irrelevant when it's a buffer.

Okay, I suppose the path of least resistance then is to make them work according to the documentation.

[bnoordhuis]

#31766