Piping streams into SHA3 without end: false crashes

@mcollina

Version: 12.4.0
Platform: Windows 10 x64 and Linux 4.4.0-042stab138.1
Subsystem: crypto or deps

This code causes a segmentation fault in 12.4.0:

const crypto = require('crypto');
const fs = require('fs');

const myOwnCode = fs.createReadStream(__filename);
const copy = fs.createWriteStream(`${__filename}.copy`);
const hash = crypto.createHash('sha3-512');
myOwnCode.pipe(hash);
myOwnCode.pipe(copy).on('finish', () => {
  hash.digest();
});

This seems to be caused by pipe calling hash._flush when end is not set to false. This code also causes a segmentation fault:

const crypto = require('crypto');

const hash = crypto.createHash('sha3-512');
hash._flush(() => console.log('Flushed'));
hash.digest();

This seems to be at least partially caused by the implementation of _flush:

node/lib/internal/crypto/hash.js

Lines 54 to 57 in 908292c

    
           Hash.prototype._flush = function _flush(callback) { 
        
             this.push(this[kHandle].digest()); 
        
             callback(); 
        
           };

It bypasses the this[kState][kFinalized] safeguard:

node/lib/internal/crypto/hash.js

Lines 79 to 91 in 908292c

    
           Hash.prototype.digest = function digest(outputEncoding) { 
        
             const state = this[kState]; 
        
             if (state[kFinalized]) 
        
               throw new ERR_CRYPTO_HASH_FINALIZED(); 
        
             outputEncoding = outputEncoding || getDefaultEncoding(); 
        
             if (normalizeEncoding(outputEncoding) === 'utf16le') 
        
               throw new ERR_CRYPTO_HASH_DIGEST_NO_UTF16(); 
        
             // Explicit conversion for backward compatibility. 
        
             const ret = this[kHandle].digest(`${outputEncoding}`); 
        
             state[kFinalized] = true; 
        
             return ret; 
        
           };

Note that this bug only happens when using SHA3, sha256 seems to be working just fine, so there might also be some weirdness in OpenSSL.

cc @mcollina @nodejs/crypto @nodejs/streams

Stack trace:

Thread 1 "node" received signal SIGSEGV, Segmentation fault.
0x00000000016a11ad in SHA3_absorb ()
(gdb) bt
#0  0x00000000016a11ad in SHA3_absorb ()
#1  0x00000000015ae107 in sha3_final ()
#2  0x0000000001593d3f in EVP_DigestFinal_ex ()
#3  0x0000000000a87230 in node::crypto::Hash::HashDigest(v8::FunctionCallbackInfo<v8::Value> const&) ()
#4  0x0000000000b9cae6 in v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) ()
#5  0x0000000000b9e9f9 in v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) ()
#6  0x0000000001a93942 in Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit () at ../../deps/v8/src/builtins/base.tq:1866
#7  0x00000000019fd584 in Builtins_InterpreterEntryTrampoline () at ../../deps/v8/src/builtins/base.tq:107

Valgrind info for the first example:

==4491== Invalid read of size 8
==4491==    at 0x21311C7: SHA3_absorb (keccak1600-x86_64.s:387)
==4491==    by 0x203230D: sha3_final (m_sha3.c:120)
==4491==    by 0x201772E: EVP_DigestFinal_ex (digest.c:171)
==4491==    by 0xE4305D: node::crypto::Hash::HashDigest(v8::FunctionCallbackInfo<v8::Value> const&) (node_crypto.cc:4640)
==4491==    by 0xFB4290: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:157)
==4491==    by 0xFB7213: v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:109)
==4491==    by 0xFB8A38: v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) (builtins-api.cc:139)
==4491==    by 0xFB956C: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (builtins-api.cc:127)
==4491==    by 0x1C81D1F: ??? (builtins-async-iterator-gen.cc:256)
==4491==    by 0x1A05334: ??? (builtins-string-gen.cc:1971)
==4491==    by 0x19EF05B: ??? (builtins-internal-gen.cc:700)
==4491==    by 0x1A05334: ??? (builtins-string-gen.cc:1971)
==4491==  Address 0x9ed8dd0 is 0 bytes after a block of size 400 alloc'd
==4491==    at 0x4C2FDFB: malloc (vg_replace_malloc.c:309)
==4491==    by 0x204422F: CRYPTO_malloc (mem.c:222)
==4491==    by 0x2044262: CRYPTO_zalloc (mem.c:230)
==4491==    by 0x20175AB: EVP_DigestInit_ex (digest.c:129)
==4491==    by 0xE42BEF: node::crypto::Hash::HashInit(char const*) (node_crypto.cc:4585)
==4491==    by 0xE42B02: node::crypto::Hash::New(v8::FunctionCallbackInfo<v8::Value> const&) (node_crypto.cc:4573)
==4491==    by 0xFB4290: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:157)
==4491==    by 0xFB4E6F: v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<true>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:109)
==4491==    by 0xFB8983: v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) (builtins-api.cc:135)
==4491==    by 0xFB956C: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (builtins-api.cc:127)
==4491==    by 0x1C81D1F: ??? (builtins-async-iterator-gen.cc:256)
==4491==    by 0x19F5799: ??? (base.tq:175)

crypto: fix crash when calling digest after piping

fix(node:crypto): hash digest survives piping; AEAD tag-length + setA…

crypto: fix crash when calling digest after piping

crypto: prevent Hmac.digest() from returning uninitialized memory

node:crypto: make Hash update()/copy() after end() throw instead of r…

crypto: prevent Hmac.digest() from returning uninitialized memory

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

Piping streams into SHA3 without end: false crashes #28245

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	Hash.prototype._flush = function _flush(callback) {
	this.push(this[kHandle].digest());
	callback();
	};

	Hash.prototype.digest = function digest(outputEncoding) {
	const state = this[kState];
	if (state[kFinalized])
	throw new ERR_CRYPTO_HASH_FINALIZED();
	outputEncoding = outputEncoding \|\| getDefaultEncoding();
	if (normalizeEncoding(outputEncoding) === 'utf16le')
	throw new ERR_CRYPTO_HASH_DIGEST_NO_UTF16();

	// Explicit conversion for backward compatibility.
	const ret = this[kHandle].digest(`${outputEncoding}`);
	state[kFinalized] = true;
	return ret;
	};

Sunbelt Computer Software

PL/B Language Development and Support

Piping streams into SHA3 without end: false crashes #28245

Description

Activity

tniessen commented on Jun 15, 2019

addaleax commented on Jun 16, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions