Another not-so-obvious bug with your example buggy code: it's also vulnerable to timing attacks.

http://codahale.com/a-lesson-in-timing-attacks/

True, but I don't think Steam allows you enough tries to make it viable.

Also not a bug, but putting strlen() inside the loop condition makes my eyes twitch

A "goto fail" style bug could allow for blankness but not truncatedness. Basic idea:

    if(!check_params()) {
        failed = true;
        goto fail;
    }
    if(some_other_check()) {
        failed = true;
        goto fail;
    }
    if(token_length == 0) {
        goto fail; // oops, forgot to set failed
    }
    ...check the token...
    fail:
    if(failed) {
        return auth_failure;
    }

I have no idea if this is anything like what actually happened, of course.

Yeah, strcmp() exists for a reason!

But in this case you'd want to use a constant-time string comparison function, which, afaik, strcmp() is not.

Is such a timing attack really possible over the internet? I'd expect the code to expire after a few failed attempt anyway.

But I guess you're right, better safe than sorry.

Surprisingly... yes, it is possible. See the link in my other comment: https://news.ycombinator.com/item?id=9953496

Good point. Is there are library of constant time functions for security sensitive applications to replace the standard ones in libc? (Seems like the kind of thing that the OpenBSD people might have written.)

strcmp() is at least better than a buggy equivalent that doesn’t actually work :)

The idea is to use constant time hashing functions, and then compare the hashes instead of comparing the strings themselves. AFAIK there is no implementation of the C stdlib that makes sure no timing attacks can be performed.

We're comparing one-time fixed-length code. Is it necessary?

Probably not. Ideally you'd implement this endpoint to invalidate the code after a few failures, so a timing attack would be impossible to carry out. But it's a good idea to use one anyway, in case it turns out that you're wrong, or that other code is also broken somehow. It costs almost nothing to do so.

Try it out here: https://jsfiddle.net/valgaze/wpL2La7d/

If this is in fact how how things worked, what a great showstopper bug

Looks like 's' is also considered valid in this example

And so is any part of the full word (s, st, ste, stea, steam)

That's one possibility. Another possibility is they were merely comparing the submitted nonce to the nonce on file. If a nonce had never been generated for a user, the database may have contained an empty string. An empty string in the db would match an empty string submitted by an attacker.

No idea or visibility into Steam's backend. Hope to god they are not using C for this very reason. Great for games, terrible for SAS

If you wrote it like that this bug would happen in every language. There is nothing in there that is worse because of C.

Yeah, but in other (more high-level) languages you simply wouldn't compare strings byte by byte.

Neither would you do it in C.

Their webapp is (AFAIK) all PHP; not sure what the backend looks like.