4 ms·

I guess this is a reminder that writing secure C is actually really, really hard.

by carey 12y ago

maxk42 12y ago
These are not problems with C. These are problems with a shitty dev team.
pyre 12y ago
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?
psychometry 12y ago
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?
- andrewchambers 12y ago
  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.
- jheriko 12y ago
  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).
  - rpcope1 12y ago
    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://www.geeksforgeeks.org/write-c-program-wont-compiler-c..., http://david.tribble.com/text/cdiffs.htm#C90-vs-CPP98 http://david.tribble.com/text/cdiffs.htm#C90-vs-CPP98)
    - jheriko 12y ago
      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.
comex 12y 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.
- cremno 12y ago
  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 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.
  - comex 12y ago
    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.
- userbinator 12y ago
  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."
kabdib 12y ago
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).

I guess this is a reminder that writing secure C is actually really, really hard.

by carey 12y ago

I guess this is a reminder that writing secure C is actually really, really hard.

maxk42 12y ago

These are not problems with C. These are problems with a shitty dev team.

pyre 12y ago

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?

psychometry 12y ago

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?

andrewchambers 12y ago
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.
jheriko 12y ago
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).
- rpcope1 12y ago
  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://www.geeksforgeeks.org/write-c-program-wont-compiler-c..., http://david.tribble.com/text/cdiffs.htm#C90-vs-CPP98 http://david.tribble.com/text/cdiffs.htm#C90-vs-CPP98)
  - jheriko 12y ago
    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.

comex 12y 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.

cremno 12y ago
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 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.
- comex 12y ago
  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.
userbinator 12y ago
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."

kabdib 12y ago

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).