Hacker News new | ask | show | jobs
by comex 4090 days ago
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.
2 comments

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.