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

Timing attack #31

Closed
RobThree opened this issue Mar 8, 2019 · 2 comments
Closed

Timing attack #31

RobThree opened this issue Mar 8, 2019 · 2 comments

Comments

@RobThree
Copy link

RobThree commented Mar 8, 2019

Your code is vulnerable to a timing attack because you use a basic string comparison. You should use hash_equals or, if you need/want to support older versions than 5.6, a simple loop that iterates the entire string, regardless of the result like this (disclaimer: author here).

Both your references have either addressed the issue or still have an active issue:

dochne pushed a commit that referenced this issue Mar 12, 2019
@dochne
Copy link
Owner

dochne commented Mar 12, 2019

Seems legit, implemented and pushed out 👍

@dochne dochne closed this as completed Mar 12, 2019
@RobThree
Copy link
Author

RobThree commented Mar 12, 2019

Out of curiosity: where'd you find the substr_count solution or did you come up with it yourself? It could be safe (I guess it is) but I do have a little doubt; not sure yet. In any way, the code doesn't reflect any means of 'timing attack safe comparison', maybe rename isEqual to something better reflecting what it does or at least add a little comment about what the substr_count 'magic' is doing there and why? Just my $0.02.

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

No branches or pull requests

2 participants