Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"crypto.createCipher()._flush" results in an abort #37922

Closed
zyscoder opened this issue Mar 26, 2021 · 3 comments
Closed

"crypto.createCipher()._flush" results in an abort #37922

zyscoder opened this issue Mar 26, 2021 · 3 comments
Labels
crypto Issues and PRs related to the crypto subsystem. known limitation Issues that are identified as known limitations. stream Issues and PRs related to the stream subsystem.

Comments

@zyscoder
Copy link

What steps will reproduce the bug?

Setup a node instance,

» node

and run the following javascript code.

crypto.createCipher("aes-256-ccm","",{authTagLength:8})._flush(function(err,data){})

Then an abort occurs.

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

This abort can always be triggered following the steps above.

What is the expected behavior?

I'm not sure this problem is of crypto or streams, but I still suggest setting these internal APIs as private fields or something like that to prevent misuse. If any error occurs, an exception or other similar error-reporting stuff should be thrown. There is no reason to abort the whole node process.

What do you see instead?

» node
> crypto.createCipher("aes-256-ccm","",{authTagLength:8})._flush(function(err,data){})
node[358058]: ../src/node_crypto.cc:4120:bool node::crypto::CipherBase::Final(node::AllocatedBuffer*): Assertion `(1) == (EVP_CIPHER_CTX_ctrl(ctx_.get(), 0x10, auth_tag_len_, reinterpret_cast<unsigned char*>(auth_tag_)))' failed.
 1: 0xa03530 node::Abort() [node]
 2: 0xa035ae  [node]
 3: 0xb25c04 node::crypto::CipherBase::Final(node::AllocatedBuffer*) [node]
 4: 0xb2e764 node::crypto::CipherBase::Final(v8::FunctionCallbackInfo<v8::Value> const&) [node]
 5: 0xbe369b  [node]
 6: 0xbe4c46  [node]
 7: 0xbe52c6 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [node]
 8: 0x13ff259  [node]
[1]    358058 abort (core dumped)  node

Additional information

@Ayase-252
Copy link
Member

Ayase-252 commented Mar 26, 2021

As the relevant docs says

This function MUST NOT be called by application code directly. It should be implemented by child classes, and called by the internal Readable class methods only.

Since you are calling Cipher._flush (Cipher extends Transform) directly from application code, it violates precondition and behavior is not guaranteed.

Historically, there is not so-much built-in OOP language features in JavaScript as other language like C++. We could "emulated" private fields by Symbol field in recent practices. But given the importance of stream module, the request to refactor(or semantic changes? considering "access modifier" is never enforced) may be assessed very carefully.

@PoojaDurgad PoojaDurgad added the crypto Issues and PRs related to the crypto subsystem. label Mar 26, 2021
@aduh95 aduh95 added known limitation Issues that are identified as known limitations. stream Issues and PRs related to the stream subsystem. labels Mar 26, 2021
@addaleax
Copy link
Member

This is conceptually the same thing as #37884. I'd suggest closing one of them as a duplicate.

@aduh95
Copy link
Contributor

aduh95 commented Apr 1, 2021

Duplicate of #37884

@aduh95 aduh95 marked this as a duplicate of #37884 Apr 1, 2021
@aduh95 aduh95 closed this as completed Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. known limitation Issues that are identified as known limitations. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants