jayce0071 / crypto-js

Automatically exported from code.google.com/p/crypto-js
0 stars 0 forks source link

bug in cipher-core.js #96

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
There is a bug in cipher-core.js that causes decryption to fail (in all 
"serialized" cases, as far as i can tell).

See line 704:

var plaintext = cipher.createDecryptor(key, 
cfg).finalize(ciphertext.ciphertext);

should be: 

var plaintext = cipher.createDecryptor(key, cfg).finalize(ciphertext);

There is no such property "ciphertext.ciphertext".

Original issue reported on code.google.com by dkent...@gmail.com on 21 Sep 2013 at 2:05

GoogleCodeExporter commented 9 years ago
"ciphertext" will be a CiperParams object (or at least it's supposed to be; 
JavaScript doesn't let us enforce types, so it's up to you to pass the right 
thing), and CipherParams objects have a property "ciphertext".

Original comment by Jeff.Mott.OR on 21 Sep 2013 at 2:36

GoogleCodeExporter commented 9 years ago
All i know is that when I made the change that i described above, the code 
worked.  Before the change, it didn't.

Original comment by dkent...@gmail.com on 21 Sep 2013 at 9:08

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Look at the _parse() function.  In the case that a string is passed into 
decrypt() for the ciphertext param (which is legal according to the decrypt() 
docs), _parse() returns a word array, not a CipherParams as _parse() is 
documented to do.  Hence the bug.  decrypt assumes it always returns a 
CipherParams object.

Original comment by dkent...@gmail.com on 21 Sep 2013 at 9:29

GoogleCodeExporter commented 9 years ago
How about you post your code that reproduces the issue.

Original comment by Jeff.Mott.OR on 21 Sep 2013 at 9:35

GoogleCodeExporter commented 9 years ago
I think the code will only work, when passing in a string, if the string is 
"OpenSSL-compatible"

Original comment by dkent...@gmail.com on 21 Sep 2013 at 9:36

GoogleCodeExporter commented 9 years ago
Just pass in any string that is not OpenSSL.  This is clear just from a cursory 
look at the _parse() function.  But if it helps, here is the call i'm making:

cryptography.decrypt = function (aString, key) {

var ivlen = this.AesOptions.blockSize; // in 32-bit words

var messageWordArray = this.base64Decode(aString);

this.AesOptions.iv = 
CryptoJS.lib.WordArray.create(messageWordArray.words.slice(0, ivlen));

var message = 
CryptoJS.lib.WordArray.create(messageWordArray.words.slice(ivlen));

return CryptoJS.AES.decrypt(message, key, 
this.AesOptions).toString(CryptoJS.enc.Utf8);
};

message is a word array.

Original comment by dkent...@gmail.com on 21 Sep 2013 at 9:41

GoogleCodeExporter commented 9 years ago
OK, more info, as i wasn't correct in my analysis above.  My code is passing in 
a WordArray for the ciphertext param, which _parse() treats as a CipherParams 
object, even though it is not.  So this appears to be the source of the error:  
I should be passing in either a CypherParams object or a string.  The docs are 
clear about this, so it looks like it is my mistake.

I apologize for wasting your time and being presumptuous!

Original comment by dkent...@gmail.com on 21 Sep 2013 at 9:54

GoogleCodeExporter commented 9 years ago
OK, I spoke too soon, again.  I find that if I pass in a Base64 string (having 
set the cfg.format to be Base64), the _parse() function returns a wordarray, 
not a CipherParams object.  As I noted above, when passing in a string _parse() 
will return a CipherParams object only when the string format is OpenSSL.

So this does appear to be a bug in either the decrypt() code or the 
documentation, as decrypt() is documented to take any string, but in fact will 
not work unless the string is formatted as OpenSSL.

Original comment by dkent...@gmail.com on 21 Sep 2013 at 10:21

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
OK, it seems like the correct fix would be to make format.parse() always return 
the same type of object.  

But I don't have the scope of knowledge to understand the implications of such 
a change. So until a fix is released I'm going with the following hack:

        _parse: function (ciphertext, format) {
            if (typeof ciphertext == 'string') {
                var retval = format.parse(ciphertext, this);
                if (format != CryptoJS.enc.OpenSSL) {
                    retval = CipherParams.create({ ciphertext: retval, salt: null });
                }
                return retval;
            } else {
                return ciphertext;
            }
        }

Original comment by dkent...@gmail.com on 21 Sep 2013 at 10:44

GoogleCodeExporter commented 9 years ago
Encoders (under the enc namespace) and cipher text formatters (under the format 
namespace) don't do the same job and are not interchangeable. You can't pass 
Base64 as a format and expect it to work. 

Original comment by Jeff.Mott.OR on 22 Sep 2013 at 12:34

GoogleCodeExporter commented 9 years ago
ok, thanks for that info.  So, as documented and coded, decrypt() will take 
either a CypherParam or a string.  Since the only format strategy type I see is 
OpenSSL, then without writing one's own custom format strategy, the string 
*must* be in OpenSSL format.  Am I missing other format types or documentation?

Given that my starting point is a WordArray, then my options are:

1) convert the WordArray to a CipherParam,
2) convert the WordArray to an OpenSSL string, 
3) convert the WordArray to another sort of string (Base64, for example) and 
write a custom format strategy that would enable decrypt() to convert the 
string to a CipherParam.

Converting the WordArray to an OpenSSL string (option 2) presets a problem in 
that the OpenSSL formatter wants "Base64" to be defined, and it is not clear to 
me without further research how to make that be defined.

I am reluctant to convert the WordArray to a CipherParam (option 1) because 
CipherParam seems to potentially overlap with the config options that I'm 
passing, which makes me uncomfortable. But the only property it seems to care 
about in this case is ciphertext.  So the code to convert WordArray to 
CipherParam looks like this:

   message = CryptoJS.lib.CipherParams.create({ ciphertext: message });

That works.  Not all very intuitive nor documented, but i'll take it.  Thanks, 
Jeff, for your help!

Original comment by dkent...@gmail.com on 22 Sep 2013 at 3:40