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

"cipher.final()" results in an abort #38035

Closed
zyscoder opened this issue Apr 2, 2021 · 6 comments
Closed

"cipher.final()" results in an abort #38035

zyscoder opened this issue Apr 2, 2021 · 6 comments
Assignees
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.

Comments

@zyscoder
Copy link

zyscoder commented Apr 2, 2021

What steps will reproduce the bug?

Setup a node instance,

» node

and run the following javascript code.

crypto.createCipher('aes-192-ccm',"",{authTagLength:10}).final()

Then the node instance occurs an abort.

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?

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
Welcome to Node.js v14.15.1.
Type ".help" for more information.
> crypto.createCipher('aes-192-ccm',"",{authTagLength:10}).final()
node[145172]: ../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]    145172 abort (core dumped)  node                                                                                                                                                                                                               

Additional information

@zyscoder
Copy link
Author

zyscoder commented Apr 2, 2021

The crash traces look the same as #37922

@Ayase-252 Ayase-252 added the crypto Issues and PRs related to the crypto subsystem. label Apr 2, 2021
@aduh95
Copy link
Contributor

aduh95 commented Apr 2, 2021

Duplicate of #38015. Although it's a different method, it's clearly the same underlying bug, or at least it must be related.

@aduh95 aduh95 closed this as completed Apr 2, 2021
@aduh95 aduh95 added duplicate Issues and PRs that are duplicates of other issues or PRs. stream Issues and PRs related to the stream subsystem. and removed stream Issues and PRs related to the stream subsystem. duplicate Issues and PRs that are duplicates of other issues or PRs. labels Apr 2, 2021
@aduh95
Copy link
Contributor

aduh95 commented Apr 2, 2021

Actually, final is not a method from the stream API as I incorrectly thought. It's probably a different bug.
I can reproduce on master.

The failed assertion is located here:

CHECK_EQ(1, EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_AEAD_GET_TAG,
auth_tag_len_,
reinterpret_cast<unsigned char*>(auth_tag_)));

@aduh95 aduh95 reopened this Apr 2, 2021
@aduh95 aduh95 added the confirmed-bug Issues with confirmed bugs. label Apr 2, 2021
tniessen added a commit to tniessen/node that referenced this issue Apr 5, 2021
tniessen added a commit to tniessen/node that referenced this issue Apr 5, 2021
@tniessen tniessen self-assigned this Apr 5, 2021
@hergetr
Copy link

hergetr commented Apr 26, 2021

Hi, I just spent some hours to find a bug in my program. Before filing the bug report I searched for similar and came to this here.

Additional Information:

  1. Same problem applies to crypto.createDecipheriv(...) which is not depricated.
  2. In my case, the string was not empty. It was a valid hex-string, but ended with '\r' (carriage-return) (which was hard to find because node aborted without the nodejs-stackdump)
  3. There is a workaround until fixed:
    • This crashes:
      decipher.write(encrypted, 'hex')
    • This works:
      const buf = Buffer.from(encrypted,'hex')
      decipher.write(buf)

Thank you very much !
Rainer

@Ayase-252
Copy link
Member

@hergetr

Hi, thanks for report.

It sounds similar to #38015. Could you compare your error stack with that issue? If it is not same, please fill a new issue.

@hergetr
Copy link

hergetr commented Apr 26, 2021

@Ayase-252 :
Yes, same error, but I had a valid hex-string + '\r'.
It was valid for Buffer.from(..,'hex') which seems to have different behavor:

// a) Buffer's hex converter works:
Buffer.from('af34\r','hex') // works. It seems to trim the input.

// b) crypto's hex-converter fails:
const decipher = crypto.createDecipheriv(algorithm, key, iv)
decipher .write('af34\r','hex') // seems not to trim and aborts with Assertion `str->Length() % 2 == 0 ...

So, if you are there changing and afterwards testing the code anyway, maybe you could consider whether to add the trim() like Buffer.from() does? The nastyness of the whitespace was, that You cannot easily see it in the textfile where the '\r\n' comes from :-)

For completenes, here is my abort message.
node[5944]: ../src/string_bytes.cc:433:static v8::Maybe node::StringBytes::StorageSize(v8::Isolate*, v8::Localv8::Value, node::encoding): Assertion `str->Length() % 2 == 0 && "invalid hex string length"' failed.
1: 0xa04200 node::Abort() [node]
2: 0xa0427e [node]
3: 0xadd70a node::StringBytes::StorageSize(v8::Isolate*, v8::Localv8::Value, node::encoding) [node]
4: 0xb3a95e node::StringBytes::InlineDecoder::Decode(node::Environment*, v8::Localv8::String, node::encoding) [node]
5: 0xb3b7be void node::crypto::Decodenode::crypto::CipherBase(v8::FunctionCallbackInfov8::Value const&, void ()(node::crypto::CipherBase, v8::FunctionCallbackInfov8::Value const&, char const*, unsigned long)) [node]
6: 0xbe56eb [node]
7: 0xbe6c96 [node]
8: 0xbe7316 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [node]
9: 0x14012f9 [node]
Aborted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants