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

Stack overflow when validating big base64 strings #502

Closed
GuillaumeLeclerc opened this issue Feb 18, 2016 · 14 comments · Fixed by #503
Closed

Stack overflow when validating big base64 strings #502

GuillaumeLeclerc opened this issue Feb 18, 2016 · 14 comments · Fixed by #503

Comments

@GuillaumeLeclerc
Copy link

Hello,

I'm facing a stack overflow when validating a base64 string. Do you think we could make validation asynchronous and use timeouts to clear the stack ?

Thank you

@chriso
Copy link
Collaborator

chriso commented Feb 18, 2016

Can you paste a snippet of the code you're running? The function is a simple non-recursive one so can't cause a stack overflow.

@GuillaumeLeclerc
Copy link
Author

Indeed the code is not recursive you are right. The recursion is in RegExp.test (native). Maybe the regex you use is evil ?

@chriso
Copy link
Collaborator

chriso commented Feb 18, 2016

If there's excessive back-tracking (ReDoS) then your app will hang rather than fail with a stack overflow. Can you paste the string you're trying to validate?

@GuillaumeLeclerc
Copy link
Author

Not sure github will allow me to paste this amount of data. I'll make a gist in a minute

@chriso
Copy link
Collaborator

chriso commented Feb 18, 2016

What's the length of the base64 string?

@GuillaumeLeclerc
Copy link
Author

more than 5mb

@chriso
Copy link
Collaborator

chriso commented Feb 18, 2016

I'm assuming the base64 string was not valid?

The regular expression we use is:

/^(?:[A-Z0-9+\/]{4})*(?:[A-Z0-9+\/]{2}==|[A-Z0-9+\/]{3}=|[A-Z0-9+\/]{4})$/i

If an equals sign is found somewhere before the end, or the last few characters are not valid, then the regex would have to backtrack. I haven't seen a stack overflow before though.

@GuillaumeLeclerc
Copy link
Author

The base64 is valid. Maybe not using a regex for validation for long strings would be better ?

@chriso
Copy link
Collaborator

chriso commented Feb 18, 2016

Happy to accept a PR 😄 Validating 5MB strings isn't the typical case.

@GuillaumeLeclerc
Copy link
Author

I understand it's not the typical case. Maybe this encoding is not suited for this amount of data

@chriso
Copy link
Collaborator

chriso commented Feb 18, 2016

In the mean time you can use this workaround (assuming you're using node):

validator.extend('isBase64', function (str) {
  return str === new Buffer(str, 'base64').toString('base64');
});

@chriso
Copy link
Collaborator

chriso commented Feb 19, 2016

@GuillaumeLeclerc could you try #503?

@GuillaumeLeclerc
Copy link
Author

Yes I'll try today if I have some time

@GuillaumeLeclerc
Copy link
Author

Thank you !

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