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

{Hash, Hmac}.digest and sign hard crash when supplied 'ucs2' encoding #9817

Closed
deian opened this issue Nov 27, 2016 · 0 comments
Closed

{Hash, Hmac}.digest and sign hard crash when supplied 'ucs2' encoding #9817

deian opened this issue Nov 27, 2016 · 0 comments
Labels
crypto Issues and PRs related to the crypto subsystem.

Comments

@deian
Copy link
Member

deian commented Nov 27, 2016

  • Version: 6.4.0 - 8.0.0-pre
  • Platform:
  • Subsystem:

Several hash functions hard crash when supplied 'ucs2' encodings.

Snippet for Hash:

    crypto.createHash('sha256').digest('ucs2');

Snippet for Hmac:

    crypto.createHmac('sha256', 'w00t').digest('ucs2');

This is because for both the binding layer functions end up calling
StringBytes::Encode with UCS2, which has a hard check:

  CHECK_NE(encoding, UCS2); // <- this can be controlled from JS
  CHECK_LE(buflen, Buffer::kMaxLength);

The Sign::SignFinal binding function does this too, but the js wrapper always
calls it with the encoding set to null. So you'd have to call handle directly
to crash:

    const private_key = '-----BEGIN EC PRIVATE KEY-----\n' +
      'MHcCAQEEIF+jnWY1D5kbVYDNvxxo/Y+ku2uJPDwS0r/VuPZQrjjVoAoGCCqGSM49\n' +
      'AwEHoUQDQgAEurOxfSxmqIRYzJVagdZfMMSjRNNhB8i3mXyIMq704m2m52FdfKZ2\n' +
      'pQhByd5eyj3lgZ7m7jbchtdgyOF8Io/1ng==\n' +
      '-----END EC PRIVATE KEY-----\n';
    crypto.createSign('RSA-SHA256')._handle.sign(crypto._toBuf(private_key), 'ucs2');

I'm not sure if this was intentional for sign.sign() (to be always called with
null encoding), but I suspect not.

+@mlfbrown for joint work.

@mscdex mscdex added the crypto Issues and PRs related to the crypto subsystem. label Nov 27, 2016
addaleax added a commit to addaleax/node that referenced this issue Apr 29, 2017
Throw `Error`s instead of hard crashing when the `.digest()` output
encoding is UTF-16.

Fixes: nodejs#9817
anchnk pushed a commit to anchnk/node that referenced this issue May 6, 2017
Throw `Error`s instead of hard crashing when the `.digest()` output
encoding is UTF-16.

Fixes: nodejs#9817
PR-URL: nodejs#12752
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
gibfahn pushed a commit that referenced this issue Jun 18, 2017
Throw `Error`s instead of hard crashing when the `.digest()` output
encoding is UTF-16.

Fixes: #9817
PR-URL: #12752
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
gibfahn pushed a commit that referenced this issue Jun 20, 2017
Throw `Error`s instead of hard crashing when the `.digest()` output
encoding is UTF-16.

Fixes: #9817
PR-URL: #12752
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 11, 2017
Throw `Error`s instead of hard crashing when the `.digest()` output
encoding is UTF-16.

Fixes: #9817
PR-URL: #12752
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants