In general I agree, we have a long way to go to writing more secure software by default.

But this specific issue is different. The issue was not the string length was incorrect (the length passed to strncmp was the correct length for the response string); the problem here was a logic problem, which is harder to get right just by switching string implementations.

The buggy code in question was basically (in an imaginary language with fully managed strings):

    if response is_a_prefix_of expected_hash:
      allow_login()

    if response is_equal_to expected_hash:
      allow_login()

The problem is that strncmp is in fact a prefix-comparison function when used with n calculated one way, but also it's the "safer" version of strcmp when used with n calculated another way, and it's easy to confuse the two uses.

In any other string implementation, you'd just use the most obviously-named compare function, there would be no need for a "safer" version of that function (there'd just be one reasonable comparison function), and it would do the right thing.

This isn't true; first off it isn't "safer" than strcmp() at all (this isn't strcpy vs strncpy after all), and second, no length argument to strncmp() will make it act like strcmp(). In order to get strcmp(), you have to also check the lengths are the same first.

    strlen(a) == strlen(b) && strncmp(a, b, strlen(either))

I agree the names are not ideal (in fact I write my own streq() and strpfx() routines because I think it reads easier and it is also easy to get the return value of strcmp() wrong), but again, that has nothing to do with separating the string's pointer and its length.

An API where experienced users don't even agree what the function is supposed to be is a bad API.

Really you want both a length, of the data actually in the string, and a capacity, marking how much memory is valid.

If I'm understanding the vulnerability right: strncmp takes three arguments, the beginning of the first string, the beginning of the second string, and the maximum number of bytes to compare. That maximum isn't quite a length or a capacity. It's a bound on the length, if one of the strings isn't null-terminated. But if you provide too small a maximum, it'll only compare the first few characters, and return a value based on that.

In particular they compared the target string to the user-provided string, with a user-provided length, so if you provide an empty string, it compares 0 bytes and returns success.

An API of the form strcmp(struct actual_string a, struct actual_string b) wouldn't have this problem - the actual string structures (instead of a char pointer) would provide a length for both strings, so the API wouldn't let you make this sort of error.

It's more convenient and safer to have these two variables packed together as a "string object" and have string functions operate on that - then they always know where the string starts and ends in memory.

With them you don't have to worry about lack of null termination as the n determines the length of the string.

Problem here was usage of them at all. Hashes are not really strings, memcmp should have been used or even better secure (that doesn't leek timings) memcmp should be used.

Problem here was just mistake in the program not in the C lib.

"More secure" doesn't imply "not dangerous". It's definitely safer to drive a car without seatbelts than to ride a bike down the highway, but that doesn't mean that not wearing your seatbelt isn't dangerous.

> Problem here was usage of them at all. Hashes are not really strings, memcmp should have been used or even better secure (that doesn't leek timings) memcmp should be used.

This is incorrect for two reasons.

The first is that hashes in ASCII format are strings: it's useful to want, for example, case-insensitive hash comparisons.

The second is that (if I'm understanding the vulnerability correctly) the exact same bug would be present with memcmp:

    if (memcmp(target, user_string, user_string_length) == 0)

Yes. But a library that people make mistakes with all the time is dangerous. If you are a programmer who ever makes mistakes (and clearly the AMT folks are, but I'm pretty sure so is everyone), you should avoid dangerous libraries.