cargo-fuzz findings

27 Feb 2017

After the announcement of cargo-fuzz last week, I decided to try using it to search for bugs in capnproto-rust. For a long time I had been meaning to try out, a similar fuzz-testing tool about which I had heard lots of good things, but its nontrivial setup costs were enough to deter me. Fortunately, cargo-fuzz is very simple and fast to get running. It quickly found several issues in capnproto-rust, all of which have been fixed in the latest release.

Panic on invalid input (fixed in 6f9bbdca)

The original C++ code suggests that the branches for dealing with Far pointers in total_size() and copy_pointer() are unreachable, so I had translated them into panics. It turns out that these branches can in fact be reached due to invalid input, rather than any bugs in the code. In the C++ version, this means that a somewhat misleading exception will be thrown; I’ve submitted a pull request to make the exception more accurate. The problem is more serious in the Rust version, because Rust makes a sharp distinction between panics and error Results.

CPU amplifications (fixed in e89f162b, 66def413)

One way to attempt to mount a denial-of-service attack on a consumer of Cap’n Proto messages is to carefully craft messages that could trick the consumer into doing lots of work. For example, if you send a cyclic structure, the consumer might go into an infinite loop trying to read it. To protect against such attacks, message readers in capnproto-rust have an adjustable traversal limit, indicating how many bytes are allowed to be read before an error is returned. Reads of zero-sized structs should also count against this limit, as was observed in these bug reports for capnproto-c++ in 2015. I thought that I had completely updated capnproto-rust with fixes for this problem, but apparently I had missed two cases. :-(

Panics on TODOs (fixed in 4c8d5f3, 77dc713b, 10f37267, 3521d4e2)

The fuzzer managed to find some explicit panics in capnproto-rust that were filling in holes of unimplemented functionality. I had forgotten that such holes still existed. After cargo-fuzz found these, I went ahead and implemented the functionality. At first I was too lazy to write a test case to cover the new code, but before going to bed that night I did set up cargo-fuzz to run on it. By the next morning, cargo-fuzz had found a memory safety issue in the new code! Even better, the test case it generated was rather clever and gave me a good starting point for writing the test that I later added to the capnproto-rust test suite to cover the new functionality.

-- posted by dwrensha

capnproto-rust on github
more posts