scrypt-js icon indicating copy to clipboard operation
scrypt-js copied to clipboard

fix: don't allow NaN/Infinity params

Open ChALkeR opened this issue 4 years ago • 3 comments

I believe this is a regression in https://github.com/ricmoo/scrypt-js/commit/e55eb3983ae3f6dc9c56b6c88f4e2d788e7edab1#diff-714a4bf03391301f9f8a3a4b026cd1a82fa422b664209252422963d9548341c6L255-R255, previous versions of scrypt-js correctly rejected NaN input but 3.x doesn't.

Also, perhaps more checks are needed there, e.g. for the number to be non-negative (>= 0) and <= Number.MAX_SAFE_INTEGER. Wdyt?

Tests included.

Without the patch, the lib misbehaves on NaN and Infinity input.

ChALkeR avatar Aug 06 '21 05:08 ChALkeR

Keep in mind that changing var to const/let will break this in older environments, so those can’t be made without a major version change.

I am planning to make a major version change in the near future though, which will allow modern syntax.

ricmoo avatar Aug 06 '21 05:08 ricmoo

@ricmoo I don't understand, sorry. 3.x is already using const/let in both the package source and tests.

The only thing that was changed outside of tests in this PR is Number.isInteger usage.

ChALkeR avatar Aug 06 '21 06:08 ChALkeR

Oh!! Sorry, I just checked and you are correct. I think I confused this with my AES library?

I'm planning a major release soon, which will port the library to TypeScript. I'll look into this PR as soon as I can.

Thanks! :)

ricmoo avatar Aug 06 '21 07:08 ricmoo