LWN: Comments on "Stagefrightening"

Stagefrightening

ortalo — Tue, 04 Aug 2015 09:11:51 +0000

IMHO, we need much more work like yours.
I have tried to be an advocate of this need for several years now (possibly decades), but not very successfully to say the least. The fact that it is a difficult technical topic (arcane for the general public) and that many people in (or more precisely around) computer security either dream of or claim impossible things certainly does not help.

But anyway, Coccinelle + Smatch, even if you factor in the compiler writers efforts, Coverity and one time research things like Astree, that's not enough (it spans a decade...).

Note that while thinking about it, that's a pretty general problem wrt computer security: investment is extremely ill-managed. Look at all those ordinay users happily paying every month for antivirus software (now for their Android smartphone also), at governemental or private funding for legal study of the cyberspace or cyberwarfare attack tools and all the difficulties you have actually funding decently security communication libraries, static analysers development, compilers enhancement, etc.

We lack some public authority with the capability to evaluate objectively these state of facts on computer security issues at macro levels. (Similar to what CERTs do at the elementary vulnerability level or maybe IETF on Internet issues.)

Governance is our problem now in this field. (Which is the elegant way to say that the people who have the money are inadequate.)

Stagefrightening

kleptog — Mon, 03 Aug 2015 21:08:18 +0000

> > It's not these these features don't exist, it's that (a) they're not standardised

>Why would this matter for, at least, the kernel?

For the kernel it doesn't matter so much, other than it's new (GCC 5.0 new to be precise). But you could probably whip these up in an afternoon in assembly if you wanted to, I think it's telling that this hasn't happened. You don't need compiler support to make these functions, it just makes it easier. The kernel devs could have implemented it years ago if they wanted to.

For all user space applications not being standardised makes it hard because you'd really rather not rely on special compiler features.

Stagefrightening

jezuch — Mon, 03 Aug 2015 07:51:40 +0000

My experience with static analyzers is that they protect you from silly mistakes. Like using instanceof on a variable that can never refer to an object of this type. Or that this field is accessed from these methods but is inconsistently synchronized. Which is great but kind of underwhelming. I don't think I've seen anything that would detect serious architectural problems.

Stagefrightening

mathstuf — Sun, 02 Aug 2015 13:03:19 +0000

Yep, much better interface there since you don't waste the computation.

> It's not these these features don't exist, it's that (a) they're not standardised

Why would this matter for, at least, the kernel?

Stagefrightening

PaXTeam — Sun, 02 Aug 2015 11:31:40 +0000

how about something like these instead:

http://clang.llvm.org/docs/LanguageExtensions.html#checke...
https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Built...
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61129

Stagefrightening

kleptog — Sun, 02 Aug 2015 11:14:14 +0000

In gcc you can just say: if(__builtin_add_overflow(foo, bar, &sum)) { error("Overflow"); }

See: https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Built...

Other compilers have similar features: https://msdn.microsoft.com/en-us/library/windows/desktop/...

It's not these these features don't exist, it's that (a) they're not standardised and (b) people aren't using them.

Stagefrightening

peter-b — Sun, 02 Aug 2015 08:43:36 +0000

> Also we often use integer overflows to test for integer overflows like this "if (foo + bar < foo)".

When doing code reviews, I usually like to see "if (foo > UINT32_MAX - bar)", for example, because it detects overflows a priori rather than a posteriori. On the other hand, it requires you to use the correct limit macro. I agree with mathstuff that a "__builtin_will_overflow()" in the compiler would be nice to have.

Stagefrightening

spender — Sun, 02 Aug 2015 02:59:02 +0000

Another big problem (maybe added under the "safe" overflow category) when dealing with integer overflows via GCC plugins is dealing with the various translation units in GCC that introduce (intentional) integer overflows not present in the source code. This has been the main source of false-positives in the past for PaX's size_overflow plugin.

-Brad

Stagefrightening

mathstuf — Sat, 01 Aug 2015 23:56:32 +0000

> Also we often use integer overflows to test for integer overflows like this "if (foo + bar < foo)".

Sounds like something worth a __builtin_will_overflow function to explicitly denote such uses.

Stagefrightening

error27 — Sat, 01 Aug 2015 09:57:28 +0000

The article suggests that this bug was probably an integer overflow. If you yse the `git log --all-match --grep integer --grep overflow --no-merges` on the kernel source then I am among the winners (Dan Carpenter).

1 25 Author: Thomas Meyer <thomas@m3y3r.de>
2 25 Author: Dan Carpenter <dan.carpenter@oracle.com>
3 24 Author: Xi Wang <xi.wang@gmail.com>
4 9 Author: Dan Carpenter <error27@gmail.com>
5 7 Author: Guenter Roeck <linux@roeck-us.net>
6 5 Author: Wenliang Fan <fanwlexca@gmail.com>
7 5 Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
8 4 Author: Haogang Chen <haogangchen@gmail.com>
9 4 Author: Brian Norris <computersforpeace@gmail.com>
10 4 Author: Andrey Ryabinin <a.ryabinin@samsung.com>

Thomas Meyer used a Coccinelle script to switch calls from kzalloc() to kcalloc(), it's a good thing to do for code hardening, but it's not bug fixes. Xi Wang was doing PHD work at MIT (http://pdos.csail.mit.edu/~xi/).

I used custom Smatch (http://smatch.sf.net) checks to find my integer overflow bugs. Even though the Smatch checks were useful, I haven't released them because they have too many false positives. Of course, a bad person could easily spend a week looking through false positives and feel good if he found one real bug, but for a normal dev that would be a waste of time. These things are asymmetric. :/

Basically, there are few static checkers which can find integer overflows. It's harder than finding buffer overflows. You have to know which data can be trusted an which comes from untrusted sources. You have to track two variables instead of one. You have to do cross function analysis. There are a lot of integer overflows which we don't care about. There are some integer overflows which are safe. Also we often use integer overflows to test for integer overflows like this "if (foo + bar < foo)".

Out of open source static checkers, Smatch comes the closest, but it is still too rough and it has only been tuned for the linux kernel. Btw, there are some runtime integer detection tools. PaX has done some work with this and Xi Wang.

Stagefrightening

ortalo — Fri, 31 Jul 2015 15:46:16 +0000

Kudos to you. And when you become the master, do not forget to impose on all your padawans the rules needed to share this excitement.
After all, programs writing programs are at the heart of our art... ;-)

immoral or amoral? It's all about perspective

pr1268 — Fri, 31 Jul 2015 12:38:16 +0000

I dunno... Myself, not knowing there was a difference in the two words' meaning, looked each up. Slightly bending the definition of amoral from "neither moral nor immoral" to being both, then amoral would fit just fine.

Granted, from the device user's perspective, intentionally neglecting to patch a vulnerability in the interest of new device sales would surely be considered immoral. From Google's senior management team, it could be considered moral.

Of course, this could also be extended to the wireless providers—their complicity in this matter is certainly ~~moral~~ I mean ~~immoral~~ Er, ~~amoral~~ just pick one! ☺

Stagefrightening

jezuch — Fri, 31 Jul 2015 09:45:36 +0000

> If I were more cynical, I might make suggestions that a lot of the "engineers" writing this code would consider such tools to "get in their way" and are even an affront to their "programming prowess".

I am not one of those "engineers". I get excited every time findbugs (static analysis tool for Java) adds a new detector and immediately start scanning all of the code under my care :) The same for pedantic warning settings in the compiler.

Stagefrightening

NAR — Fri, 31 Jul 2015 07:56:41 +0000

The problem is that static analyzers don't make the code flawless. I remember a "sales pitch" for an Erlang product, they said that practically all of the reported bugs were the kind of "software did what the developer wanted to do, not what the user wanted to do". Static analyzers may decrease the number of some kind of bugs, but definitely not eliminate all bugs. So the choice is between "slower development, less features, (hopefully) less bugs" and "faster development, more features, (probably) more bugs".

Stagefrightening

ms — Thu, 30 Jul 2015 18:34:54 +0000

Why is that a poor story? Tools are awkward and tricky to use until they get regular use and get polished by that. Remember how awful the first editors were?
The tools will get better once they are mandated for any code that is ever fed untrusted inputs. I'd rather have fewer features and slower code but code that is flawless than a billion insecure and badly implemented features. Slowly, more people may come to agree with this.

Stagefrightening

jhoblitt — Thu, 30 Jul 2015 16:58:01 +0000

There's no shortage of static analysis tools out there. However, it's a pretty poor story to tell developers if you just run these 10+ tools, and manually pick through the massive number of false positives, your code is _less likely_ to have a major security issue.

https://en.wikipedia.org/wiki/List_of_tools_for_static_co...

The analysis tools and languages need to evolve significantly to keep the development work-flow at a reasonable level of complexity. Start holding your breath...

Corporations only understand liability

brugolsky — Thu, 30 Jul 2015 16:04:33 +0000

The solution to the problem of embedded firmware is liability throughout the manufacturing and distribution chain. The only safe harbor should be independently buildable open source, subject to a "standard of care" that requires little more than a USB/Bluetooth/WiFi/... connection to the device and "git clone ... && cd ... && make build install push" or similar. That should include fully user-replaceable keys for all secured components. Manufacturers that are fond of putting DRM in their products should state in appropriate documentation and marketing literature that user-modified software that impairs DRM will disable specific functionality.

Stagefrightening

ortalo — Thu, 30 Jul 2015 13:24:40 +0000

It's also heavily uneffective: you *may* get some corrections but you *probably* get more new vulnerabilities too.

In fact, sometimes it seems the sure-fire way to get a security improvement is to downgrade to an old dumb phone; though this does nothing to try to solve the problem of course.

Stagefrightening

ortalo — Thu, 30 Jul 2015 13:18:35 +0000

I am not as optimistic as you. (Seriously!)

It seems to me massive problems have already occurred but culprits consistently manage to get out of harm and, sooner or later, secure computer systems may become luxury equipment.
Not that model checking cannot be done for yachting in the Carribean but it would leave me with a sense of guilt. ;-)

Stagefrightening

ms — Thu, 30 Jul 2015 12:59:44 +0000

s/amoral/immoral/. Learn something new every day...

Stagefrightening

ms — Thu, 30 Jul 2015 12:41:35 +0000

Cute idea :) Struggling to think of one western government where > 1% of the ministers know what Android is though...

Stagefrightening

magnus — Thu, 30 Jul 2015 12:37:08 +0000

Maybe the manufacturers are hoping that Android is "too big to fail" and that they then get their costs for patching all old devices (or replacing with new ones) covered by a govenment contract. :)

Stagefrightening

ms — Thu, 30 Jul 2015 11:49:55 +0000

It certainly benefits them and their partners that one of the only sure-fire ways to get a security update is to have to drop $500 on a new phone. The extent to which this is unethical and amoral beggars belief.

Stagefrightening

Fowl — Thu, 30 Jul 2015 11:41:17 +0000

Many fuzzers actually are much closer to "compile" time, eg. http://lcamtuf.coredump.cx/afl/

The update model for Android is just broken. Decades of experience in hardware independent software has been thrown out the window in the name of expediency. Worse is better, eh?

Stagefrightening

ms — Thu, 30 Jul 2015 10:03:51 +0000

If I were more cynical, I might make suggestions that a lot of the "engineers" writing this code would consider such tools to "get in their way" and are even an affront to their "programming prowess". The fact peer review didn't apparently catch this stuff either is terrifying. But I'm sure they probably did TDD or at least have 95%+ code coverage in their tests. So that makes everything OK.

As the produce of our industry becomes ever more critical in maintaining every aspect of civilisation, we have *got* to get better at *proving* correctness, or at the very least vast model checking. People who don't understand tools like quickcheck or similar are going to rapidly find themselves unemployable. As the recent Jeep issue shows (cars could be rooted and controlled remotely), sooner or later there are going to be massive financial consequences to getting this stuff wrong.

Stagefrightening

ortalo — Thu, 30 Jul 2015 09:31:37 +0000

I wonder if such things cannot be discovered by directly using the static analysis functions of compilers...
I am always surprised that fuzzing is the most popular tool nowadays, even when you have access to the source code.
In some sense, fuzzing is just "instrumented luck" no? As opposed to static analysis, which would be "rigorous verification". (Yes, I am deliberately polemic.)

Anyway when privileged libraries get deployed to a billion device, I would expect the supplier to request controls in the build system. (I imagine Linux is not built with gcc -w.) Brown paper bag for Google & co. IMHO.
Unless all those smartphones are really disposable devices and all their users knowingly agree...