Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RegExp used to detect numbers is vulnerable to ReDoS #777

Closed
FarSeeing opened this issue Apr 2, 2020 · 3 comments · Fixed by #779
Closed

RegExp used to detect numbers is vulnerable to ReDoS #777

FarSeeing opened this issue Apr 2, 2020 · 3 comments · Fixed by #779

Comments

@FarSeeing
Copy link
Contributor

Basically the title says everything.

Node code to test:

const Papa = require('papaparse');

const input = '0'.repeat(10000);
const options = {dynamicTyping: true};

let start = process.hrtime.bigint();
Papa.parse(input, options);
let end = process.hrtime.bigint();
console.log(`Numerical input: ${Number(end - start) / 1e9} seconds`);

start = process.hrtime.bigint();
Papa.parse(input + 'a', options);
end = process.hrtime.bigint();
console.log(`Non-numerical input: ${Number(end - start) / 1e9} seconds`);

Results on my PC:
For 10000 characters:

Numerical input: 0.002068456 seconds
Non-numerical input: 0.574908789 seconds

For 100000 characters (note the exponential growth):

Numerical input: 0.002522945 seconds
Non-numerical input: 57.080913533 seconds

This comes from var FLOAT = /^\s*-?(\d*\.?\d+|\d+\.?\d*)(e[-+]?\d+)?\s*$/i; and the issue is in (\d*\.?\d+|\d+\.?\d*) segment.

One possible solution - replace current RE with something like /^\s*-?(\d+\.?|\.\d+|\d+\.\d+)(e[-+]?\d+)?\s*$/. Looks ugly but it works.

@pokoli
Copy link
Collaborator

pokoli commented Apr 2, 2020

Thanks for opening the issue. I've just added the regexp on #779
Could you please have a look on it?

@FarSeeing
Copy link
Contributor Author

Yes, looks good to me. Thanks.

pokoli added a commit that referenced this issue Apr 2, 2020
@pokoli
Copy link
Collaborator

pokoli commented Apr 2, 2020

I've uploaded version 5.2.0 to npm which contains this bug fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants