Security and Tools

Everybody should remember than a 100% secure device is the one unplugged and put in a safe covered in concrete. There is always a trade-off on the impairment we inflict ourselves in order to stay safe.

Antonio Lioy

In the wake of the heartbleed bug. I’d like to return again on what we have to track problems and how they could improve.

The tools of the trade

Memory checkers

I wrote in many places regarding memory checkers, they are usually a boon and they catch a good deal of issues once coupled with good samples. I managed to fix a good number of issues in hevc just by using gcc-asan and running the normal tests and for vp9 took not much time to spot a couple of issues as well (the memory checkers aren’t perfect so they didn’t spot the faulty memcpy I introduced to simplify a loop).

If you maintain some software please do use valgrind, asan (now also available on gcc) and, if you are on windows, drmemory. They help you catch bugs early. Just beware that sometimes certain versions of clang-asan miscompile. Never blindly trust the tools.

Static analyzers

The static analyzers are a mixed bag, sometimes they spot glaring mistakes sometimes they just point at impossible conditions.
Please do not put asserts to make them happy, if they are right you just traded a faulty memory access for a deny of service.

Other checkers

There are plenty other good tools from the *san family one can use, ubsan is maybe the newest available in gcc and it does help. Valgrind has plenty as well and the upcoming drmemory has a good deal of interesting perks, if only upstream hadn’t been so particular with release process and build systems you’d have it in Gentoo since last year…

Regression tests

I guess everybody is getting sick of me talking about fuzzy testing or why I spent weeks to have a fast regression test archive called playground for Libav and I’m sure everybody in Gentoo is missing the tinderbox runs Diego used to run.
Having a good and comprehensive batch of checks to make sure new code and new fixes do not have the uncalled side effect of breaking stuff is nice, coupled with git bisect makes backporting to fix issues in release branches much easier.

Debuggers

We have gdb, that works quite well, and we have lldb that should improve a lot. And many extensions on top of them. When they fail we can always rely on printf, or not

What’s missing

Speed

If security is just an acceptable impairment over performance in order not to crash, using the tools mentioned are an acceptable slow down on the development process in order not to spend much more time later tracking those issues.

The teams behind valgrind and *san are doing their best to just make the execution three-four times as slow when the code is instrumented.

The static analyzers are usually just 5 times as slow as a normal compiler run.

A serial regression test run could take ages and in parallel could make your system not able to do anything else.

Any speed up there is a boon. Bigger hardware and automation mitigates the problem.

Precision

While gdb is already good in getting you information out of gcc-compiled data apparently clang-compiled binaries are a bit harder. Using lldb is a subtle form of masochism right now for many reasons, it getting confused is just the icing of a cake of annoyance.

Integration

So far is a fair fight between valgrind and *san on which integrates better with the debuggers. I started using asan mostly because made introspecting memory as simple as calling a function from gdb. Valgrind has a richer interface but is a pain to use.

Reporting

Some tools are better than other in pointing out the issues. Clang is so far the best with gcc-4.9 coming closer. Most static analyzers are trying their best to deliver the big picture and the detail. gdb so far is incredibly better compared to lldb, but there are already some details in lldb output that gdb should copy.

Thanks

I’m closing this post thanking everybody involved in creating those useful, yet perfectible tools, all the people actually using them and reporting bugs back and everybody actually fixing the mentioned bugs so I don’t have to do myself alone =)

Everything is broken, but we are fixing most of it together.

6 thoughts on “Security and Tools”

  1. > Please do not put asserts to make [static analyzers] happy, if they are right you just traded a faulty memory access for a deny of service.

    No! Crashing the app is far superior to allowing random memory access to an attacker. In fact, crashing is the best default behaviour of any program in case of an otherwise unhandled error. If you care for availability the crash can be caught and the component restartet.

    1. No, it is not “superior”, any bug is equally damaging depending on the context.

      A crash is much more annoying than having the last line of an hd video garbled.

      Feeding back cleantext from a tls connection you don’t own is quite more problematic if the content might be a sensitive password. Yet if that server just provide public information and uses tls just speed up, having it crash would just cause a larger damage.

      Every time you look at a bug you should spend time deciding which is the impact.

      But either the issue causes a real problem (like heartbleed) or even requiring a CVE for every malformed input unhandled is just spamming.

    2. No a crash strategy is _NEVER_ fine and “fixing” something you do not understand fully is yet another recipe for bigger issues in the future.

      Trading a bug for a bug just to please a paranoid tendency is WRONG and “hey, let’s do something really quick so the compiler is pleased” is even worse.

      This approach is what got a previous lovely OpenSSL bug in Debian.

      You have to FIX the bugs, eventually, all of them. I explained quite verbosely in another post. Kostya explained in his blog why some would have lower priority because time isn’t infinite.

      But if you care, you do fix it yourself or you donate money to the cause.

      I spent a decent chunk of time of my spare time to get a great deal of mistakes fixed, some silly, some much more interesting. Never took shortcuts.

  2. > Every time you look at a bug you should spend time deciding which is the impact.

    Assessing whether a bug is exploitable is often too expensive for the programmer. It is also effort invisible to every reviewer of the written/corrected code. In retrospect many innocently looking bugs turned out to be exploitable.

    > A crash is much more annoying than having the last line of an hd video garbled.

    Fine example. A crash-early strategy works perfectly here – as long as you remember to recover from the crash somewhere. E.g. crash the frame decoder and continue with the next frame or keyframe. If a crash really is no option, than handle the error. Please don’t let some cat video corrupt my memory.

  3. I fully agree with everything you say in your last post except the introducing sentence. Crashing (and recovering from a crash) is a de-facto standard pattern for highly available systems, whereas trying to repair at failure point is not[1].

    Back to the initial point: If a static analyzer thinks code is performing a faulty memory access and if the programmer thinks it is not than this is a prime spot for an assertion. If the programmer is right it comes at no cost (except in the most performance critical sections). If the static analyzer is right, the assertion transformed a potential DoS+Leak+Code Injection into a DoS with a highly informative debug message. Because invalid memory access yields DoS (e.g. by accessing an unmapped page) and code injection (writing outside of the destined region).

    Of course there are situations in which the programmer can proof the code is safe despite a static analyzer warning. But this proof is usually much more expensive than adding an assertion. And in practice it is wrong, all to often.

    And of course a better error handling strategy than assert() can be employed where appropriate. But pretty-printing errors is much less important than securing exploitable bugs.

    [1] https://www.usenix.org/legacy/events/hotos03/tech/candea.html

    1. You move the cleanup up one level because might be simpler to do that (and as for every pattern like that you could be completely wrong and you’ll pay later ^^).

      You have an impossible condition that might happen. Oxymoron aside, the static analyzer tells you that.

      You can do the extra miles and figure out if you can make that condition just really impossible fixing the code where the problem actually is or:

      – Be lazy

      if (nearly_impossible_condition)
      return BUG;

      – Be dangerously lazy

      if (nearly_impossible_condition)
      abort();

      – Be a fool

      assert(!nearly_impossible_condition);

      The latter is being a fool since asserts should be used _ONLY_ when developing in my opinion.

      I’m not fond of the angel approach, somebody sufficiently dedicated can just make your worker crash often enough that your system would just waste resources bringing them up just to see them fail again. Additional fun if the angel doesn’t have a rate-control and bringing up a worker takes more resources than keeping it functioning.

Leave a Reply

Your email address will not be published.