Hacker News Reader

Comment by meithecatte | Hacker News Reader

I don't think it's strncmp itself that's the problem. They fucked up and are only checking that the provided password matches a prefix of the stored password. And obviously an empty string is a matching prefix.

View on HN

The (decompiled) code looks very amateurish in general. Like, who writes something like system("echo >... or system("sync"). Looks like something written by a very green engineer/intern and not reviewed by anyone else.

Sadly this looks like "industry standard" to me. I have seen many embedded Linux devices using mostly C applications and a lot of usage of the system() function with no or very poor input validation before. When you want to exploit embedded Linux devices always look for calls to system() first.

The "strncmp(saved_pwd,pass0,strlen(pass0))" looks equally bad. Probably someone did not understood the advice "always check the length first" and just did it everywhere.

Intel AMT checked the password in a similar way some time ago: https://www.tenable.com/blog/rediscovering-the-intel-amt-vul...

Many of these faults are simply the consequence of never reading the manpage of a function but making assumptions about implementation details of an imaginary version of a function with the same name.

Though, in this case, manpage doesn't exactly spell out that two strings of which one is 0 chars long results in a match.

I often remember my PHP days in horror, but mysqli_query manpage does warn you about SQL-injections now.

It does actually.

"The strncmp() function is similar, except it compares only the first (at most) n bytes of s1 and s2."

Possibly it's years of string wrangling in C that sets me up here for being biased towards the compact way in which the C manpages state what the function will do, but 'early termination' because of zero length strings for comparisions returns 0 just as sure as comparing "" and "" would. And that 0 indicates a match...

Of course an experienced developer would have thought about edge cases, but I can see a thought process like: ok, I'm comparing two strings. "" != "123" in languages with built-in string types.

I'm definitely not arguing that C shouldn't be used and everybody should be using <insert-the-currently-trendy-systems-programming-language>: just thinking out loud if improved documentation could prevent at least some of the common footguns.

Embedded Linux engineers in general are not incompetent. Many of us do care about quality of the software we write. Although you are right that there is a lot of dodgy embedded software around, that isn't a reason to accept mediocrity like this in newly written user facing APIs.

I have never seen firmware from any vendor in any industry that doesn't use these patterns. There's a lot to unpack culturally around "hardware company engineers" vs "software company engineers," but I don't think this is "amateurish" so far as it is "industry standard from another industry."

Reply on news.ycombinator.com