Files with certain sizes cause 'Uncaught RangeError: Maximum call stack size exceeded ' with SHA1

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Run this: dd if=/dev/urandom of=2test.raw bs=$((1024 * (124) + 128 + 57 )) 
count=1 
2. Load file as ArrayBuffer (in a web worker context; might not be important)
3. Pass to CryptoJS.SHA1() wrapped in CryptoJS.lib.WordArray.create()
4. Print resulting SHA1 with toString()

What is the expected output? What do you see instead?

SHA1 of input file.  Instead I see:

'Uncaught RangeError: Maximum call stack size exceeded'

Note: adding or subtracting a few bytes from the random file makes SHA1 work...

What version of the product are you using? On what operating system?

3.1.2, Ubuntu 12.04 LTS 64 Bit, Chrome 24.0.1312.52

Please provide any additional information below.

Original issue reported on code.google.com by wolfgang...@gmail.com on 23 Jan 2013 at 11:26

[GoogleCodeExporter]

I did not verify all byte sizes in between, but file sizes of 256KB work fine 
as well (power of 2, larger than this magic size).  Adding a few bytes still 
fails (mistaken in the original report).

Original comment by wolfgang...@gmail.com on 23 Jan 2013 at 11:29

[GoogleCodeExporter]

Just so you don't have to do math, the file size I notice this starting at is: 
127161 bytes :-)

Original comment by wolfgang...@gmail.com on 23 Jan 2013 at 11:32

[GoogleCodeExporter]

It would help if you could attach the code that reproduces the error.

Original comment by Jeff.Mott.OR on 23 Jan 2013 at 11:40

[GoogleCodeExporter]

Attached.

Original comment by wolfgang...@gmail.com on 23 Jan 2013 at 11:48

Attachments:

• example.tar.gz

[GoogleCodeExporter]

Hi, wolfgang. I generated a file with the steps you suggested, and I used the 
example page you attached, but I couldn't reproduce the error. Is there perhaps 
some code missing from the example page that would make the difference?

Original comment by Jeff.Mott.OR on 27 Jan 2013 at 4:01

[GoogleCodeExporter]

The HTML is exact, and I think the JavaScript is too.  I'm still getting the 
error, it says the exception hits on line 8 of sha1.js.

I'm going to try a different browser and see if that makes a difference.

Original comment by wolfgang...@gmail.com on 27 Jan 2013 at 7:07

[GoogleCodeExporter]

I can reproduce this issue with web workers.

- Chrome, Version 33.0.1750.117
- Ubuntu 12.04, 64 Bit

I am getting the *Uncaught RangeError: Maximum call stack size exceeded* for 
calculationg the SHA-1 hash values for Uint8Arrays with a length from 127029 to 
262140 Bytes.

Screenshot: http://paste.dennis-boldt.de/bugs/chrome/cryptojs/screenshot.png
Example: http://paste.dennis-boldt.de/bugs/chrome/cryptojs/
The worker: http://paste.dennis-boldt.de/bugs/chrome/cryptojs/task.js

Original comment by dennis.b...@googlemail.com on 25 Feb 2014 at 3:52

[GoogleCodeExporter]

Yep, I just checked on modern Chrome and still hit this issue.

Original comment by wolfgang...@gmail.com on 25 Feb 2014 at 4:03

[GoogleCodeExporter]

I wrote one more test without workers:

http://paste.dennis-boldt.de/bugs/chrome/cryptojs/test_without_worker.html

This works fine. Even for the magic range.

Original comment by dennis.b...@googlemail.com on 25 Feb 2014 at 4:20

[GoogleCodeExporter]

Found out this is a core issue with Chrome allowing small stacks for web 
workers:

https://code.google.com/p/chromium/issues/detail?id=252492

They cross-reference this bug report (and one other on crypto-js).

Trying to understand what you're doing at line 226 in the core.js file to see 
if  a workaround is possible.

I used the non rolled up versions to inspect the code easier and find out 
exactly what is failing.

This is the line that gets the error for me (I'm not sure if my comment there 
is globally visible, if it is I can delete it):

https://code.google.com/p/crypto-js/source/browse/tags/3.1.2/src/core.js#220

Original comment by wolfgang...@gmail.com on 25 Feb 2014 at 4:36

[GoogleCodeExporter]

Is there any good reason _why_ this line:

https://code.google.com/p/crypto-js/source/browse/tags/3.1.2/src/core.js#220

uses '.push.apply' instead of _concat_ ?  Why not use the JavaScript built-in 
array function concat instead of this harder to read (and WebWorker buggy) 
version?

Original comment by wolfgang...@gmail.com on 25 Feb 2014 at 4:53

[GoogleCodeExporter]

This small patch fixes the issue, and I don't see any reason not to apply it?

➜ src>svn diff

Index: core.js
===================================================================
--- core.js     (revision 667)
+++ core.js     (working copy)
@@ -217,7 +217,7 @@
                 }
             } else {
                 // Copy all words at once
-                thisWords.push.apply(thisWords, thatWords);
+                this.words = thisWords.concat(thatWords);
             }
             this.sigBytes += thatSigBytes;

Original comment by wolfgang...@gmail.com on 25 Feb 2014 at 5:36

[GoogleCodeExporter]

I think that 'push.apply' is definitely the wrong idiom to use here, as you 
_can not predict_ the stack size of a target browser's JavaScript 
implementation.

As far as I can tell, 'push.apply' creates a function call with _arguments_ 
equal to the number of elements in an array in this case.

This will clearly fail at different thresholds on different implementations.

However, I think that '.concat' will not fail for arbitrarily sized arrays up 
until OOM (basically, you should be safe with all browsers and JavaScript 
runtimes).

I hope my small patch can be applied soon :-)  I could finally use crypto-js 
with this!

Original comment by wolfgang...@gmail.com on 25 Feb 2014 at 6:37

[GoogleCodeExporter]

Hey Wolfgang,

I just saw, that you traced the bug. That's really cool. I will try to run my 
code with your small patch :)

Original comment by dennis.b...@googlemail.com on 14 Mar 2014 at 1:28

[GoogleCodeExporter]

Even Jeff had already some problems with thisWords.push.apply:

https://code.google.com/p/crypto-js/source/detail?r=666

Original comment by dennis.b...@googlemail.com on 14 Mar 2014 at 1:32

[GoogleCodeExporter]

Yep, glad to see more evidence.

Can't believe I returned to this after a year, but I do like CryptoJS :-)

Original comment by wolfgang...@gmail.com on 14 Mar 2014 at 2:29

[GoogleCodeExporter]

Great! r666 fixes this bug!  All we need is a new release.  I'm closing this as 
solved for now, as we have r666, just not a new release.

Original comment by wolfgang...@gmail.com on 14 Mar 2014 at 2:54

[GoogleCodeExporter]

I had the same "Maximum call stack size exceeded' error, and r666 fixes the 
bug!!  Thanks!  Any plan to have a new CryptoJS release? v3.1.2 does not 
include this fix.

Original comment by wor...@gmail.com on 11 Sep 2014 at 2:20