dochne/google-authenticator

Timing attack

RobThree opened this issue · 2 comments

RobThree commented

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 commented

Seems legit, implemented and pushed out 👍

RobThree commented

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.