DISALLOW_REUSE line not correctly maintained in pam module #26

Closed
ThomasHabets opened this Issue Oct 10, 2014 · 5 comments

Comments

Projects
None yet
2 participants
Contributor

ThomasHabets commented Oct 10, 2014

Original issue 26 created by valient on 2011-01-18T08:24:54.000Z:

Noticed that DISALLOW_REUSE was not being correctly maintained. To more easily demonstrate the problem, I extracted invalidate_timebased_code into a separate test program and called it multiple times in a loop, here is the progression:

ABABABABABABABA
" DISALLOW_REUSE 43177960
" TOTP_AUTH
1234567

ABABABABABABABA
" DISALLOW_REUSE 43177960 43177961
" TOTP_AUTH
1234567

ABABABABABABABA
" DISALLOW_REUSE 43177960 43177961 43177962
" TOTP_AUTH
1234567

ABABABABABABABA
" DISALLOW_REUSE 43177960 43177961 43177962 43177963
" TOTP_AUTH
1234567

ABABABABABABABA
" DISALLOW_REUSE43177961 43177962 43177963 43177964
" TOTP_AUTH
1234567

ABABABABABABABA
" DISALLOW_REUSE43177961 43177962 43177963 43177964 43177965
" TOTP_AUTH
1234567

ABABABABABABABA
" DISALLOW_REUSE4317796143177963 43177964 43177965 43177966
" TOTP_AUTH
1234567

... It continues to get worse every 2 iterations.

My wrapper code used for testing:

int main(int argc, char **argv)
{
char *buf = strdup("ABABABABABABABA\n"
"" DISALLOW_REUSE\n"
"" TOTP_AUTH\n"
"1234567\n");

int tm = get_timestamp();
for(int i=0; i<10; ++i, ++tm) {
invalidate_timebased_code(tm, &buf);
}

return 0;
}

google was assigned by ThomasHabets Oct 10, 2014

Contributor

ThomasHabets commented Oct 10, 2014

Comment #1 originally posted by valient on 2011-01-18T08:36:04.000Z:

Looks like the following change to pam_google_authenticator.c fixes it:

@@ -439,6 +440,7 @@
// remove it from the file.
if (blocked - tm > 3 || tm - blocked > 3) {
endptr += strspn(endptr, " \t");

  •  ptr += strspn(ptr, " \t");
    
    memmove(ptr, endptr, strlen(endptr) + 1);
    memset(strrchr(ptr, '\000'), 0, endptr - ptr + 1);
    } else {
Contributor

ThomasHabets commented Oct 10, 2014

Comment #2 originally posted by vittgam on 2011-02-27T16:29:39.000Z:

I've fixed this issue at https://github.com/VittGam/google-authenticator following comment 1 suggestion.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #3 originally posted by markus@google.com on 2011-03-09T20:35:17.000Z:

<empty>

Contributor

ThomasHabets commented Oct 10, 2014

Comment #4 originally posted by markus@google.com on 2011-03-09T20:35:51.000Z:

Issue 28 has been merged into this issue.

Contributor

ThomasHabets commented Oct 10, 2014

Comment #5 originally posted by markus@google.com on 2011-03-10T08:47:21.000Z:

This is now tested for in the unittest and fixed in the code. Thank you for the very detailed bug report and for even attaching a patch. I ended up fixing things a little differently, but the patch helped in explaining the bug.

google was unassigned by ThomasHabets Oct 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment