SHA-512 (maybe others) not functioning correctly on Safari

[bmurray]

Some examples, using this code (and the demo code on the main site):

var shaObj = new jsSHA(passcat, "ASCII"); var hash = shaObj.getHash("SHA-512", "HEX");

Input: passcat = "test:test"; Ruby: 8b537fa1b7db5287ab508e3a108f1a7be11058e37476ca6507a574feb02e71e7e70ffb14657beaae3600c86a71048700f4d41e08996ccb5a18686edbe6cf7e27 Chrome: 8b537fa1b7db5287ab508e3a108f1a7be11058e37476ca6507a574feb02e71e7e70ffb14657beaae3600c86a71048700f4d41e08996ccb5a18686edbe6cf7e27 Firefox: 8b537fa1b7db5287ab508e3a108f1a7be11058e37476ca6507a574feb02e71e7e70ffb14657beaae3600c86a71048700f4d41e08996ccb5a18686edbe6cf7e27 Safari: 9e743c45c0a06847e3efaad33664429598f0af24ad43ccf9db18179cc07c3d402adb1309111ac1e89c21dd2d0a5ddff4c06b03c09c8a0b7441dbc72c1009c281

It is consistently wrong. Oddly, subsequent runs in safari return: fe8a3ee70fa2eab307736063d86b4b1f95e3bcce2310fc723b4558f64a126b9b5bfe27ab0df714d03edfea220faae8955c8592e5051b7ff9230567817304ab83

Code for subsequent runs: for (i = 0; i < 10; i++ ) { var shaObj = new jsSHA(passcat, "ASCII"); var hash = shaObj.getHash("SHA-512", "HEX"); /* Print hash */ }

[Caligatio]

Any chance you could try the newest sha_devel.js and/or sha.js that I have checked in? I'm currently away from a Safari-friendly computer for a week and am hoping somehow the committed miscellaneous tweaking fixes this.

This is truly bizarre as Chrome, IE, and Firefox all test fine...

[bmurray]

Yep, that actually did it. The current head is working rather nicely. It makes a bit of sense, as they all have their own JS engine. There was a bug in some math functions in v8 that made Lastpass unusable for Chrome several months ago. But, that seems to fix it.

[bmurray]

I spoke too soon... It's apparently pretty intermittent. After a while, it starts working, but then if I close it and reopen the browser, it stops working again. I am going to try some debugging on this end.

[Caligatio]

That is absolutely bizarre. Anything you can do to pin it down further would be much appreciated!

[bmurray]

I added a "shim" inside the "binb2hex" function that returns the result of the hash function. (basically, just print whatever gets passed in the argument).

A correctly functioning version on chrome has: [-1957462111, -1210363257, -1420784070, 277813883, -519022365, 1953942117, 128283902, -1339133465, -418383084, 1702619822, 906020970, 1896122112, -187425272, -1720923302, 409497307, -422609369]

Safari, when not functioning correctly, has: [-1636549563, -1063229369, -470832429, 912540309, -1729056988, -1388065543, -619178084, -1065599680, 719000329, 286966248, -1675502291, 173924340, -1066728512, -1668674700, 1104922412, 269075073]

So it is certainly with the hashing function itself, and not in the print function.

Also, while debugging, I realized that the error doesn't happen with the debug console open, only without it (with some rather shocking regularity). Perhaps the Safari browser does some weird threading that causes this?

I am going to keep digging... My efforts are mostly focused on "system" functions for now, and what they return that is different.

[bmurray]

Apparently, this is a widely known issue with Safari.

https://discussions.apple.com/message/18001083#18001083

I tried pidCrypt as well, and it does something very similar. Javascript console open, it works. Javascript console closed, it fails. There is some major issue on the later versions of Safari's JS engine. I think we can conclusively call this a Safari issue.

*Edit: I did a bunch of debugging as well, and found that the error seems to sit in the rotr_64 function. Its a stupid simple function, but it appears to be a 64/32 bit issue. Safari can only run in 64 bit mode now, but when run in 32 bit mode on older versions of OS X, reports are that it works as expected. I will leave that with you.

[Caligatio]

So it turns out I have no access to a 64-bit OS X machine to test this out on. Any chance you could try the latest HEAD?

[ghost]

I wrote bugreport about this bug: https://bugs.webkit.org/show_bug.cgi?id=88673 But, looks like, developers needs help. Can anyone with Mac try to help them write "an isolated test case, as minimal as possible."? thank you.

[Caligatio]

sailormax, I commented on the thread but unfortunately don't have much useful to add. I've now gotten a report that iOS6 devices may be affected as well :(

[chrisws]

Hi Caligatio -

The problem goes away when javascript debugging is enabled and there's still a problem with the latest sha code. I've got an iMAC for testing, so can offer some limited assistance to help fix the problem. Not sure if there's anything that can be fixed in JS if the runtime is broken.

Cheers, Chris

[rongarret]

Any news on this? I just encountered this problem and am about to dive into debugging it but I don't want to reinvent the wheel.

[ghost]

Based on my bug report status ( https://bugs.webkit.org/show_bug.cgi?id=88673 ) and on "Assigned To: Nobody", you can try. Try to test binary shift operators. Looks like problem in them. but I could be wrong.

[chrisws]

I've found a partial fix. The str2binb() function was not always initializing each of the elements in the bin[] array. I changed it to this:

str2binb = function (str, strLen) { var bin = [], mask = (1 << charSize) - 1, length = str.length * charSize, i; for (i = 0; i < strLen; i++) { bin[i] = 0; } ....

and passed the appropriate strLen in the couple of places it was called.

Now I get the correct results on my iMac when I call this:

var shaObj = new jsSHA(<<string longer than 39 characters>>, "ASCII");
var hash = shaObj.getHash("SHA-512", "HEX");
document.writeln(hash);

But if I call the function again like this, the results are incorrect:

shaObj = new jsSHA("1", "ASCII");
hash = shaObj.getHash("SHA-512", "HEX");
document.writeln(hash);

The undefined values returned in the bin array from the str2binb() function were causing the W[] variable in coreSHA2() to gradually take on incorrect values in the nested for loop (t=0 to 80).

This might be enough for some people to get by, but I need the function to work the second time to salt the hash with the user's sessionId.

I'll let you know if I make any further progress.

Cheers, Chris

[rongarret]

I think I have found the problem. The rotr_64 function seems to mutate its argument. I do this:

rotr_64 = function (x, n) { x = new Int_64(x.highOrder, x.lowOrder); ...

and that seems to fix the problem.

[Caligatio]

Ron, if your fix successfully solves this problem, I'll owe you.

Do you need to make a copy of x before EVERY operation or just in the beginning of the function? If >>> mutates it every time, I would think I need to make a new copy before every shift.

[rongarret]

I don't know. My results are still preliminary (it was late). Copying X at the beginning of rotr_64 seems to fix sha512, but I didn't run extensive tests, and I didn't try any of the other hashes (sha512 is the only one I'm using). I was NOT able to reproduce the problem in isolation, i.e. if I just put the code for rotr_64 in a file and run that by itself it seems to work OK. Remember, this is a Safari JS bug, probably a compiler optimization bug, so it could be very complicated to really pin down. But this seems to be progress :-)

[Caligatio]

Ron, after a false start, I pushed the proposed changes up and it looks like it just worked on an affected computer I have access to for the weekend.

Can you confirm? If this works, I'm going to officially call v1.4 done.

[rongarret]

I've had some inconsistent results this morning, but at the moment it seems to be working with the fix. Since this is crypto code, I would feel a lot warmer and fuzzier if I had a full understanding of what was going on, but of course everyone has to make their own risk assessment. I'm going to keep beating on this for a while because I have some time before I have to go make the mashed potatoes :-)

[rongarret]

To give you some idea of how weird this is, I can also fix the bug like this:

rotr_64 = function (x, n) { if (n <= 32) { var foo = x.lowOrder * 1; ...

[chrisws]

Fixed....

rotr = function (x, n) { if (typeof x.lowOrder === "undefined") { x.lowOrder = 0; } if (typeof x.highOrder === "undefined") { x.highOrder = 0; }

My previous suggestion is not required.

Where's my beer?

[rongarret]

Wow, I really, really, really hate Javascript.

[rongarret]

This is enough to fix the bug too:

  var foo = typeof x;

Or this:

  var foo = typeof n;

Or this:

  var foo = typeof 1;

This has to be one of the weirdest bugs I've ever seen.

[Caligatio]

Ron: does the fix that I pushed up work or are you just experimenting for the sake of experimenting?

Chris: Do you know where the undefined's are coming from? If I need to initialize the various arrays in my *2bin functions, that seems cleaner

[rongarret]

Yes, the fix you pushed "works", but almost certainly not for the reason that you think it does. In particular, it has nothing to do with destructive changes to X, and it has nothing to do with undefined values in X. This is shown by the fact that you can "fix" the problem by adding (certain kinds of) dead code. I'm continuing to work on it just because I really want to understand what's going on here. Weird bugs usually hide interesting lessons.

[Caligatio]

Understood. I do appreciate the work you both are pouring into this wonky problem.

[rongarret]

This "fixes" the problem too:

if (0) { foo = typeof 1; }  // WTF?!?

[chrisws]

Hi Caligatio,

You can see the undefines in other browsers (I was using win7+chrome), for example when str2binb leaves uninitialised elements in the returned bin array as described previously, so you should be able to analyse this further without a mac,

Cheers, Chris

[rongarret]

Just in case anyone is still following this, I've produced a fairly small (100 LOC) test case that reliably reproduces the bug. You can find it at http://www.flownet.com/ron/weird_bug.html

It's not very user friendly, but you should be able to figure out what is going on by looking at the source. Remember, this is Safari only. The bug does not occur in Firefox, Chrome, or Opera. I have not tried IE (I don't do Windows).

I'm running Safari 5.1.7. Does anyone have a more recent version? (I don't intend to upgrade past Snow Leopard.)

[chris-nz]

I just spent two nights figuring out why my login system wasn't working on my iPhone 3GS (iOS 6.01). Turns out the SHA512 hashes PHP was generating were completely different to the jsSHA Safari was generating. Then I figured out if you do hash('sha512', 123); in PHP and var shaObj = new jsSHA(123, 'ASCII'); var hash = shaObj.getHash('SHA-512', 'HEX'); the hashes are different on every browser. Something to do with hashing it as an integer, but change the 123 to a string e.g. '123' for PHP and JavaScript code and the hashes match in most of the browsers except on my iPhone. Very strange.

[chris-nz]

I can confirm the latest version of sha_dev.js in the src directory is producing matching SHA512 hashes on iOS 6.01 and PHP. My login system is working again on iOS. :) I'll be using this for a while until you release your official 1.4 or whatever. No rush. Keep up the good work!

[rongarret]

The trick is not fixing the bug. That turns out to be easy. The trick is figuring out whether or not you can really trust the fixed code because one of the many ways to fix it is adding code that never actually runs. See http://www.flownet.com/ron/weird_bug.html

BTW, it is not surprising that you'd get inconsistent results computing the hash of an integer. That's not a well-defined operation. SHA hashes are only well defined on sequences of (8-bit) bytes.

[Caligatio]

So I nuked the v1.31 download as it was outright busted for Safari (should have done that a while ago) but now I'm stuck wondering if I should push an official v1.4 release just to make it so the code works with Safari-flavored browsers.

Anyone have any strong feelings on the matter?

[rongarret]

I would go ahead and release it, but with a prominent disclosure in the comments that it contains a bug that is fixed but not fully understood. I'd include instructions on how to reproduce the bug. Then I would not worry about it any more. Crypto in JS in the browser is pretty dodgy to begin with.

[chris-nz]

Before each release It would be really good to have some basic tests run on each of the hash algorithm variations to make sure it matches a known good working hash result e.g. on a server side language like PHP.

If you know a server side language e.g. PHP then you could generate hashes for 10 different strings for each hash algorithm and variation. For PHP you'd do it with hash() or crypt() then store all the results in a big associative array. Then you json_encode() the output to the client side JavaScript on the page. Now the JavaScript gets the hashes computed from the server side language and computes the same hashes client side. Then you compare each hash generated with the server side version and the jsSHA computed version and output a pass or fail next to each one. Results are in a big table. Then you have a running overall counter as well and output the end result as pass if all the hashes match and fail if there's even one mismatch. So once that's built, then you run it on all the main browsers you can get your hands on and see if it works. E.g. Firefox, Chrome, Opera, Safari, IE 9, Android, iOS Safari. If it's working on all then that's good to release.

[Caligatio]

Chris-nz, that's been done almost since first release (but I redid it completely in1.4).... it's in test/test.html

I test on Firefox (Linux, Windows, and Snow Leopard), Chrome (Windows, Linux, and Snow Leopard), Safari (Snow Leopard), IE 9 (32 and 64 bit), and Android (Gingerbread). I unfortunately don't/didn't have a 64-bit Mac or an iPhone which is how this problem lingered for so long.

[vip9]

Hi, v1.4 failed on Chrome (25.0.1354.0 dev) on SHA-384 and 512 Hash Tests.

[chris-nz]

Have run the tests on a few of my browsers and my iPhone 3GS. I can confirm jsSHA 1.4 passes all tests on latest release version of Chrome on Windows (Version 23.0.1271.97 m), Firefox on Windows 17.0.1 release and most importantly all tests pass on iOS 6.0.1.

Not sure there's much point investigating failed tests on dev branch of Chrome? Isn't that unstable and likely to have all kinds of random errors?

[Caligatio]

I'm using Chrome Beta v24 without any problems. If the problems crop up in beta, I'll take a closer look.

[Caligatio]

Ron, it looks like the Chrome Dev bug is related to the same rotr_64 issue (noooo). Chrome with breakpoints works, without breakpoints is broken.

[Caligatio]

Finally closing this issue for real with c7443ceafdaf84788a10e74c60c14cbbcee971c5 Apparently, under random conditions, a left bit shift will misbehave. Issue is solved by masking the post-shifted number by 0xFFFFFFFF

Thank you all for your help!

[wojwal]

Hi!

I still have the same issue with Safari - please reopen the issue.

I'm testing it using: browserstack.com (OS X Lion + Safari 5.1)

The result's I'm getting are the same as in first post...

Also like already mentioned - when I start Javascript Debugging and error console is opened - the issue is gone.

[Caligatio]

Can you try upgrading to the newest version of Safari, I believe it's 6.0.2? That's what I did my testing on and it passed with flying colors.

EDIT: Re-added the v1.4 Safari patch, could you please test the HEAD?

[wojwal]

That was a quick fix! :-)

This latest update helped. I just tested it with Safari 5.0, 5.1 and 6.0 - it all works now.

Thanks a lot

[Caligatio]

I pushed out v1.42 this morning with the unexplainable v1.4 patch readded. Sorry for the hiccup :(