Having implemented security protocols and used state machines to track state, this is a surprisingly easy (if embarrassing) mistake to make. It’s very easy to spend a lot of energy validating the documented state transitions and essentially forget to ban all other transitions.

For me this is much less “OMG how could they be so careless” and much more “There but for the grace of a diligent set of testers go I”

I guess you could say that about absolutely any bug though? This isn’t particularly underhanded, and it’s more of a broad design flaw than a subtle change. If you wanted a “backdoor” you could absolutely do 10x better than this.

How could you do 10x better? Because it's a broad design flaw there is no suspicious LOC or suspiciously overcomplicated code needed to hide the backdoor. You have to understand the whole context to notice the bug.

But it’s still there in plain sight. I’d hazard a guess that this may be the first time a solid code reviewer looked at the code, and spotted it straight away*

If you wanted 10x better, you could throw in some code that exhibits undefined (but known on major compilers) behavior, or super subtle C issues. Even a simple if statement in C forces all kinds of wonderful promotions and type conversions which can truncate, wrap, and drastically change values in ways that even most C developers aren’t aware.

I encourage folks to go check out the “underhanded C contest” before suggesting that this bug is a backdoor.

*The person who found this, Peter, is indeed insanely good.

Make sure the PRNG does not source enough entropy thus dramatically weakening all session keys to a crackable level.

Or do like WEP and recycle your one time pads. Lots of subtle design flaws one can introduce that generally get spotted in blackbox testing, and not in code.

I mean, a backdoor can just be flipping an if somewhere from && to ||, not exactly suspicious if it's mixed in with other stuff.

Pretty sure it's not insane to wonder that about any bugs _that give you a remote authentication bypass_...

Sure, you can wonder, but when I break it down, it’s either highly unlikely or highly clumsy.

If I were to put a backdoor into this code, I’d want the following properties:

- tricky to spot in code review

- virtually undiscoverable via current best-of-breed fuzzing

- tricky to spot in network captures or any type of IDS

This bug passes #2 (unless you’ve got a state-aware network fuzzer and panic() in the right places), but fails on the other two.

But who knows, maybe this was a low-investment effort and it paid off for some time, with a trivial-to-exploit (IE no mem corruption) bug they knew would eventually be retired?

Nope. It is so glaring it could only have been a mistake.

Hehe, which makes it a perfect plausible deniable mistake ;-)