nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.66k stars 29.62k forks source link

crypto Cipher/Decipher validation issues #45189

Open targos opened 2 years ago

targos commented 2 years ago

This also affects Decipher methods.

Here's an example:

const { createCipheriv, randomBytes } = require('crypto');

const cipher = createCipheriv('aes-256-cbc', randomBytes(32), randomBytes(16));

cipher.update('test', 'utf-8', 'bad');

Gives an internal assertion:

Error [ERR_INTERNAL_ASSERTION]: Cannot change encoding
This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
Please open an issue with this stack trace at https://github.com/nodejs/node/issues

final has the same issue:

const { createCipheriv, randomBytes } = require('crypto');

const cipher = createCipheriv('aes-256-cbc', randomBytes(32), randomBytes(16));

cipher.update('test', 'utf-8', 'utf-8');
cipher.final('bad');

Other issue:

const { createCipheriv, randomBytes } = require('crypto');

const cipher = createCipheriv('aes-256-cbc', randomBytes(32), randomBytes(16));

let result = cipher.update('test', 'bad', 'hex');
result += cipher.final('hex');
console.log(result);

Gives no error at all for the wrong input encoding.

@nodejs/crypto

kivancbilen commented 1 year ago

Would it make sense to introduce a new error like ERR_CRYPTO_UNKNOWN_ENCODING with a message Unknown encoding {wrong encoding name}?

panva commented 1 year ago

I'm afraid the last mentioned "other issue" is also present in Sign , Verify, Hash, Hmac, and the like. I also suspect hard validating may be breaking for a lot of legacy applications.

I suggest to deal with the first two in #45990 and leaving the inputEncoding validation to a separate PR and CITGM to decide whether it's worth fixing or not.

Also refs: #31766