Hacker News new | ask | show | jobs
by carey 4089 days ago
I guess this is a reminder that writing secure C is actually really, really hard.
5 comments

Writing completely secure C is hard, but this code is littered with extremely basic bugs like unchecked sprintf and not sanitizing arguments to system. Like, it's a basic rule that you should use snprintf instead of sprintf, possibly with exceptions for cases where you're absolutely sure the result fits in the provided buffer, and in this case there is sprintf everywhere and no checks on the input size whatsoever.
I wonder where this "length blindness" comes from, since it certainly leads to a lot of vulnerabilities. Are these programmers who started with a higher-level language than C, one with dynamically sized strings that automatically expand? Do they even know how big the buffer is, or how long the input string could conceivably be? Did they ever consider the case where the input is very, very long?

A funny analogy I've heard is "programmers who don't know the size of their buffers are like drivers who don't know the size of their cars."

Just to hazard a guess, there's probably little organizational incentive to prevent security issues in the first place.
I agree, but I don't think mentioning such an exception is a good idea. That's basically the same reason why gets() is part of ISO C90 and C99. Just call snprintf() even in such cases and forget about sprintf().

From http://www.open-std.org/jtc1/sc22/wg14/www/C99RationaleV5.10..., PDF page 163:

>The Committee decided that gets was useful and convenient in those special circumstances when the programmer does have adequate control over the input, and as longstanding existing practice, it needed a standard specification.

Fair enough... it's not like snprintf has any noticeable overhead. Sometimes I use sprintf just to indicate to readers of the code that the output is not expected to be truncated, but that's probably too cowboy for my own good.

For the record, when writing new code, I'd strongly recommend that people consider using asprintf instead of either function.

These are not problems with C. These are problems with a shitty dev team.
Or a reminder that when someone sends you a list of things wrong, maybe you should read and understand it rather than applying a 2 second fix that doesn't actually fix anything?
I don't write C at all but aren't there code analysis tools that catch things like this? Some kind of standard code linting library, maybe?
Yes, the point is that these are obvious flaws, and dlink didn't even fix them. Someone from the dlink team just sucks.

edit: And that someone may not even be the developer, at the end of the day, the company does not have the systems in place for quality control.

Yes there are these things, although usually more focused on C++ these days.

Compiling C as C++ with a C++ compiler is not a bad idea though... many compilers which will deal with both and tend not to care about pure C very much at all. Modern, extremely popular compilers may not even support C89 features yet... not to mention that lots will allow dangerous things like returning nothing from a function with a non void return type without even a compile error.

Many tools can catch the bugs mentioned here though - things like PVS studio, cppcheck, or the built in visual studio or xcode analysers (I would never recommend pc-lint, sorry), and some of these things mentioned are compiler warnings in some cases (sprintf will trigger the endlessly annoying CRT_NO_SECURE_WARNINGS message from the ms compiler for instance).

  Modern, extremely popular compilers may not even support C89 features yet...
Citation needed please. Also, because C is not a true subset of C++, it has seemed that many big C projects will not compile on C++ compilers because of corner cases, so this may be something to watch for. Much of the incompatibility rises from additions in C99.

References: (http://www.geeksforgeeks.org/write-c-program-wont-compiler-c..., http://david.tribble.com/text/cdiffs.htm#C90-vs-CPP98)

well spotted, that should have been C99, of which many features are not supported with the ms compiler. i believe that the variable array thing was pulled in C11 because so few compilers ever bothered to support it...

i see problems with it regularly where we use c code that goes through clang just fine, but cl complains. not putting declarations at the top of a file or scope is the obvious example that comes to mind and constantly snags people...

that being said there are even C++ problems, although smaller. for instance, its impossible to use the preprocessor variadic macros across cl, clang, gcc without getting warnings from at least one of them. i like to run with warnings as errors and the highest warning settings possible.

i'm not going to bother digging up references for informal conversation... but thanks for catching the mistake.

Well, maybe just one "really"

What is really hard is finding decent firmware engineers. Ones who care, and who can write secure code.

Even harder, finding a management chain that values security and that is willing to pay for it, and its continual upkeep (because security is a process, not a feature that you can complete and move on from).