Skip to content

Commit

Permalink
crypto: make update(buf, enc) ignore encoding
Browse files Browse the repository at this point in the history
Make the cipher/decipher/hash/hmac update() methods ignore the input
encoding when the input is a buffer.

This is the documented behavior but some inputs were rejected, notably
when the specified encoding is 'hex' and the buffer has an odd length
(because a _string_ with an odd length is never a valid hex string.)

The sign/verify update() methods work okay because they use different
validation logic.

Fixes: #31751

PR-URL: #31766
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
  • Loading branch information
bnoordhuis authored and MylesBorins committed Mar 9, 2020
1 parent 83e9a3e commit a727b13
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 12 deletions.
6 changes: 3 additions & 3 deletions lib/internal/crypto/cipher.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,13 @@ Cipher.prototype.update = function update(data, inputEncoding, outputEncoding) {
inputEncoding = inputEncoding || encoding;
outputEncoding = outputEncoding || encoding;

if (typeof data !== 'string' && !isArrayBufferView(data)) {
if (typeof data === 'string') {
validateEncoding(data, inputEncoding);
} else if (!isArrayBufferView(data)) {
throw new ERR_INVALID_ARG_TYPE(
'data', ['string', 'Buffer', 'TypedArray', 'DataView'], data);
}

validateEncoding(data, inputEncoding);

const ret = this[kHandle].update(data, inputEncoding);

if (outputEncoding && outputEncoding !== 'buffer') {
Expand Down
14 changes: 5 additions & 9 deletions lib/internal/crypto/hash.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,13 @@ Hash.prototype.update = function update(data, encoding) {
if (state[kFinalized])
throw new ERR_CRYPTO_HASH_FINALIZED();

if (typeof data !== 'string' && !isArrayBufferView(data)) {
throw new ERR_INVALID_ARG_TYPE('data',
['string',
'Buffer',
'TypedArray',
'DataView'],
data);
if (typeof data === 'string') {
validateEncoding(data, encoding);
} else if (!isArrayBufferView(data)) {
throw new ERR_INVALID_ARG_TYPE(
'data', ['string', 'Buffer', 'TypedArray', 'DataView'], data);
}

validateEncoding(data, encoding);

if (!this[kHandle].update(data, encoding))
throw new ERR_CRYPTO_HASH_UPDATE_FAILED();
return this;
Expand Down
22 changes: 22 additions & 0 deletions test/parallel/test-crypto-update-encoding.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict';
const common = require('../common');

if (!common.hasCrypto)
common.skip('missing crypto');

const crypto = require('crypto');

const zeros = Buffer.alloc;
const key = zeros(16);
const iv = zeros(16);

const cipher = () => crypto.createCipheriv('aes-128-cbc', key, iv);
const decipher = () => crypto.createDecipheriv('aes-128-cbc', key, iv);
const hash = () => crypto.createSign('sha256');
const hmac = () => crypto.createHmac('sha256', key);
const sign = () => crypto.createSign('sha256');
const verify = () => crypto.createVerify('sha256');

for (const f of [cipher, decipher, hash, hmac, sign, verify])
for (const n of [15, 16])
f().update(zeros(n), 'hex'); // Should ignore inputEncoding.

0 comments on commit a727b13

Please sign in to comment.