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

Fix possible regex dos vulnerability. #8

Closed
wants to merge 2 commits into from

Conversation

patsplat
Copy link

@joshbressers
Copy link

Your patch works as expected, here is me running the reproducer against the old and new packages

node test.js
Old time (milliseconds): 2175
New time (milliseconds):2

@Trott
Copy link

Trott commented Nov 5, 2020

Hi, @patsplat. Thanks for this. I don't have write access to this repo (maybe @tj / @jonathanong / @ForbesLindesay / @yields might see fit to change that?) but @tj was kind of enough to give me access to the package on npm, so I've publish 0.0.2 with this change (and then quickly published 0.0.3 because I failed to update components.json for 0.0.2). Take a look (since the code/package is very small and easily auditable) and then, if it looks good to you, maybe close this issue?

@Trott
Copy link

Trott commented Nov 5, 2020

Hi, @patsplat. Thanks for this. I don't have write access to this repo (maybe @tj / @jonathanong / @ForbesLindesay / @yields might see fit to change that?) but @tj was kind of enough to give me access to the package on npm, so I've publish 0.0.2 with this change (and then quickly published 0.0.3 because I failed to update components.json for 0.0.2). Take a look (since the code/package is very small and easily auditable) and then, if it looks good to you, maybe close this issue?

Oh, and if you want to audit the changes via GitHub, it's in my fork at https://github.com/Trott/trim.

@joshbressers
Copy link

This is awesome, thanks for the quick help all!

@joshbressers
Copy link

To clarify my previous post. I tested the new trim, it works as expected.

node test.js
Old time (milliseconds): 5
New time (milliseconds):1

@patsplat
Copy link
Author

@joshbressers thanks for validating the fix.

@tj Thanks for providing @Trott with npm access.

@Trott Thanks for patching. Looks great!

giphy-downsized-large

@patsplat patsplat closed this Nov 12, 2020
This was referenced Mar 25, 2021
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 this pull request may close these issues.

3 participants