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

CryptoJS.enc: stringify() Not Working with Word Arrays #459

Open
windsurfer1122 opened this issue Jul 3, 2023 · 3 comments
Open

CryptoJS.enc: stringify() Not Working with Word Arrays #459

windsurfer1122 opened this issue Jul 3, 2023 · 3 comments

Comments

@windsurfer1122
Copy link

windsurfer1122 commented Jul 3, 2023

When using CryptoJS.enc.Hex.stringify(decrypted) to convert the result of an AES decryption, then the result is always an empty string, although the WordArray contains 8 values.
When using CryptoJS.enc.Base64.stringify(decrypted) the exception RangeError: invalid array length is thrown.
So stringify() is not working, although using it as specified in the documentation paragraph "Encoders".
Please fix.

A special case that I have seen are negative values inside the WordArray.

My use case with CryptoJS 4.1.1 in latest Mozilla browser (incl. my workaround):

<html>

<head>
...
  <script src='https://cdnjs.cloudflare.com/ajax/libs/crypto-js/4.1.1/crypto-js.min.js'></script>
  <script type='text/javascript'>
...
    var decrypted = CryptoJS.AES.decrypt(encrypted, CryptoJS.enc.Hex.parse(aesKeyHex), { mode: CryptoJS.mode.ECB });
    // my workaround
    console.log("Hex Workaround: |" + decrypted.words.map(
      function(integer) {
        if (integer < 0) integer >>>= 0; // convert to two's complement via unsigned right shift
        return integer.toString(16).padStart(8,'0');
      })
      .join('');
    // CryptoJS issues
    console.log("Hex: |" + decrypted.toString(CryptoJS.enc.Hex) + "|");
    //console.log("Hex: |" + CryptoJS.enc.Hex.stringify(decrypted) + "|");
    console.log("Base64: |" + decrypted.toString(CryptoJS.enc.Base64) + "|");
    //console.log("Base64: |" + CryptoJS.enc.Base64.stringify(decrypted) + "|");
...
  </script>
</head>

<body onload='...'>
...
</body>

</html>

WordArray decrypted:

sigBytes: -99
words: (8) […]
​​​0: 456636238
​​​1: -1475245102   !!! negative !!!
​​​2: 1393713086
​​​3: 1032909125
​​​4: 1416189913
​​​5: 581792039
​​​6: 1598733184
​​​7: 958413955

Expected result:

1b37b74e a8118bd2 53125fbe 3d90f145 546957d9 22ad7127 5f4abb80 39203c83
@windsurfer1122
Copy link
Author

windsurfer1122 commented Jul 3, 2023

Updated the issue as it turned out that CryptoJs.enc.XXX.stringify(words); in the docs was missleading.
Instead of an array of words, a WordArray object has to be passed.
Still I get no hex or b64 string from it.

Please update the docs:

  • write CryptoJs.enc.XXX.stringify(WordArray); to avoid confusion

@windsurfer1122
Copy link
Author

I did read the docs several times again and still didn't get any clue what could be wrong.
Also the WordArray class is not described in the docs.

Decided to check out the code of CryptoJs.enc.Hex.stringify(): https://github.com/brix/crypto-js/blob/develop/src/core.js#L385
The member sigBytes of a WordArray object specifies the number of relevant/significant bytes of the included array with word integers (=signed 32-bit integers). To ignore any "padding" bytes of the last word integer.

As sigBytes is negative in my use case there cannot be any output at all. So I assume my decrypt call is incorrect.

Please update the docs:

  • describe WordArray class to make analysis/debugging easier.

@windsurfer1122
Copy link
Author

windsurfer1122 commented Jul 3, 2023

When building a CipherParams object from input/outside data, that is to be used with CryptoJS.AES.decrypt(), then it also has to include the key, although the key is specified again in the CryptoJS.AES.decrypt() call.

After changing the code (as following) the member sigBytes was correctly set and .toString() works.

var decrypted = CryptoJS.AES.decrypt(
  {
    key: aesKeyWordArray, // key has to be specified here too  WAS MISSING
    iv: null,
    salt: null,
    ciphertext: CryptoJS.enc.Hex.parse(inputHex),
  },
  aesKeyWordArray, // key has to be specified here too
  {
    mode: CryptoJS.mode.ECB,
    padding: CryptoJS.pad.NoPadding,
  }
);
console.log("Hex: |" + decrypted.toString(CryptoJS.enc.Hex) + "|");

Conclusion:
The implementation of CryptoJS is not resilient against incomplete CipherParams objects.

CryptoJS should always use only one of the specified keys.

  • for plaintext/string input: the explicitly specified key from the decrypt call.
    Of course, as there is no other.
  • for a CipherParams object: only the key inside of it.
    Ignoring the explicitly specified key from the decrypt call completely. Actually the user could specify it as null.
    If the key inside the CipherParams object is missing or incorrect, then a corresponding ValueError exception should be thrown.

Additionally the docs should be updated:

  • write CryptoJs.enc.XXX.stringify(WordArray); to avoid confusion with array of words
  • describe WordArray class in two sentences to make analysis/debugging easier (sigBytes)
  • if enhancing CryptoJs as mentioned above, then write var decrypted = CryptoJS.AES.decrypt(encrypted, null);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant