I can honestly say this: if the code or coder tells me that the client determines the length of the heartbeat separately from the heartbeat signal, I'd say its a problem. I pretty much say that for anything really.
That isn't an answer to my question.
If the server isn't calculating length from the data given then you're just asking for problems.
You mean like how every binary-safe protocol ever works, including IP itself?
Put it this way: if you had a subroutine that accepted an array of unknown length as input, would you have that subroutine also accept the length of the array as a separate variable or would you have the subroutine calculate that yourself?
It depends on the nature of the contents of the array. If it's binary data and not a null-terminated string, you
have to pass the length as a separate variable when you're working with C because there is no way to "calculate" the length of a piece of memory. All you get is a pointer; you need a size as well to know where to read up to.
You could use strlen() if you're passing a string and not binary data, but that's inefficient, particularly for large strings.
1. Reading this code is a great way to learn about encrypted connections so I'd see it being read by a lot of students.
2. As something that is vastly important to a lot of very large companies, I can see the code being poured over by people looking for exploits: both for good and not so good reasons.
That's probably true on both counts, which is why it's generally the subtler bugs that go unnoticed. In this case, all the existing code was doing the right thing; the bug was that there was missing code needed to do
all the right things. It's a lot harder to read code and know what is missing than to see mistakes in what's already there.
Put another way, reading the buggy subroutines in isolation isn't sufficient to understand the problem. You need to understand where the function's inputs are coming from and what the server state is at the time it's executed before you realise that it's not doing all the validation it should be. Not just anyone skim-reading the code would pick up on it, especially if they're not specifically looking for bugs.