node

node copied to clipboard

Fix Hash and Cipher aborting when using end with hex and specific lengths of chunks. The issue was caused because there was missing validation on the written content from the end method. Note that this actually affects quite a few things in Crypto. This affects decipher/cipher (both deprecated) but also decipheriv/cipheriv as well as Hash and Hmac.

I wasn't sure if this should throw in _transform or create an error and provide it to the callback, but according to the stream docs providing an error to the callback is what's expected - so I would love to get some input on my fix, and if it makes sense.

Fixes: https://github.com/nodejs/node/issues/38015

Does it also fix issue in https://github.com/nodejs/node/issues/38035#issuecomment-827053609? It contains invalid string (\r) in hex encoding too, however it aborts when using write.

@Linkgoron I think a better approach overall would be validating this on the C++ side instead, where the abort occurs in the first place.

Does it also fix issue in #38035 (comment)? It contains invalid string (\r) in hex encoding too, however it aborts when using write.

I checked now, and it fixes the issue in the comment - or at least it causes an error to get emitted instead of an abort.

@Linkgoron I think a better approach overall would be validating this on the C++ side instead, where the abort occurs in the first place.

Currently it appears to me that the validation in both cases (Hash and Cipher) is already done on the JS side in the update method, but maybe missed in _transform as an oversight (I'll try to go back through the PRs and see what the intent was), and I think that moving the validation to the c++ side would probably mean changing update as well.