Limit str to 100 to avoid ReDoS of 0.3s

[karenyavine]

Hey,
The fix for CVE-2015-8315 was incomplete. Limiting the input to 10,000 chars reduced the risk significantly but it is still possible to cause a delay of 0.3s. Suggested to limit it to 100 chars as there is no reason for a date to be over that :)

PoC:

ms=require('ms');
ms('1'.repeat(9998) + 'Q')
//Takes about ~0.3s

Thanks,
Karen Yavine
Security Analyst @ snyk.io

[Delagen]

#90

[leo]

Sweet! Thank you. 😊

[karenyavine]

Hey @leo!
Thanks for merging the PR!
Wondered when a new version will be released to npm?

[Delagen]

Sad that module authors does not care about performance at all.

[leo]

@karenyavine It was just released with version 2.0.0! 🎉

[grnd]

@leo, thanks for the fix and the release.
I was wondering why was this a breaking change, requiring a major release?

[leo]

Because we lowered the limit. So if people have longer strings in place, it will break.

[Delagen]

@grnd @leo I offer solution to not simply decrease limit, but to rework parsing, that boost parse speed up to 2 times, look at #90, code looks cleaner and more simple

[grnd]

+1 on @Delagen's solution. Both the previous fix of limiting to 10000 chars, and the new one of limiting to 100 chars were merely a workaround.