mruby.sh

The 500.000$ release

written by Daniel Bovensiepen on 2017-03-26 17:26:07 UTC

This story can be told in at least two ways. The first one might go like that:

The pessimistic view

An evil company decided to use mruby in one of their products. While doing this they gave their users more or less full-control of the code being executed on the VM. At one point they might have realized that this introduces a small security threat to their infrastructure. So they opened a so called bug-bounty in which they claimed to pay a certain amount of money to everyone who finds a way to exploit a machine running mruby with only access to the code being executed on the VM. The price money they planned for was up to 20.000$ in case someone found a critical bug which could compromise their infrastructure. That was around mid of November 2016. By December 2016 there were already so many bugs found that the company decided to cut down their payouts from this point to 10% of the original amount. And still the bug reports accumulated over time. Until March 2017 more than 40 security researchers reported 131 bugs and the company had to payout an astonishing 473.600$. You could call it “the worst bug bounty ever“.

The same story could also go like that:

The optimistic view

Shopify, an e-commerce shop platform from Canada, wanted to provide more flexibility to their customers. Their current Ruby On Rails App allows customers to build own shops without having to handle server and payment infrastructure by themselves. With the integration of mruby they now wanted to allow their customers to add self-written scripts to these shops for non standard functionalities. Even though that mruby is by now 5 years available as open source they might have realized that the usage is still limited (mainly by core committer like me or by early adopters e.g. game scripting engines). Relying your core infrastructure on such a piece software might have been a little bit to risky and so they apparently decide to start a bug-bounty. The payments for a reported bug was set at a range from 1.000$ to 20.000$. Over the course of several weeks it became apparent that there were A LOT OF SECURITY RELATED BUGS, so they decided to put the whole mruby instance into a sandbox (using seccomp-bpf) and reduced the payouts to 10% of the original payments as they could catch now many of the bugs before they could harm the underlying infrastructure. Yet they still left the challenge open and until today they invested essentially half a million US Dollar to improve the mruby implementation. Based on all 131 bug reports we committed 162 changes to the mruby repository and added also many regression tests to increase the chance of catching repeating bugs in the future. You could call it “the best thing to ever happen to mruby’s security”.

Post mortom

No matter which story you prefer let’s just talk about the post-mortem of this happening. After seeing the first bug reports and merge request in November 2016 by bouk I notice that Shopify started this bug bounty on HackerOne. It was definitely exciting to see that a company is willing to pay money to search and find bugs in our software. It is after all an indicator that the software you have written is useful. What else could you wish for? I saw security audits before during my professional work but the length and the amount of reported bugs of this open audit has amazed me. One reasonable explanation could be that our software quality was (is?) not high enough and has massive potential for improvement. Furthermore Ruby is a complicated language which makes every Ruby implementation complex and likely to contain bugs. As a matter of fact I notice at least one of the bugs which was found in mruby to be also applicable to standard C Ruby (MRI). Furthermore our testing infrastructure is still not good enough to catch regressions. Even though that many commits, which fixed the bugs, contained test code I notice several bugs which actually were fixed before but re-appeared after additional changes. I believe strongly that we will improve on that, because the test infrastructure itself is available and growing, what is lacking is the continuous usage of it in the case of bug-fixing. For new feature it is already since a long time common to add tests to check the behavior. We have to push this also to bug-fixing. Another point I see here is, that mruby is rapidly stabilizing. Version 1.3.0 will probably be released in the next weeks (I guess we wait until the bug storm is over) but the massive changes to the languages are actually over. One of the last remaining language features missing are keyword arguments. Most other demanded core parts are now available and stable. This also means that the risk of introducing new bugs today is much lower than before as we don’t change the core massively anymore. A risk we will still have are of cause performance improvements and refactoring of the code itself but here again regression testing will support our way forward.

Code review

Now let’s have a look at what was actually found during the bug bounty. Shopifies focus was probably to find potential bugs that could crash their infrastructure, access private data from other customers or manipulate the payment system. To be quite frankly here, all of these horror-scenarios were quite likely multiple times possible based on the security bugs found during the bug-bounty. They could provoke so many random crashes and targeted overflows that I had a hard time to even collect all the commits for this post. As a general remark it is as always: C sucks as a language to write stable code in a simple manner. That’s obvious the reason why we work on mruby so that we can replace it in specific fields. Yet for implementing a language itself, C is still the way we have to go. I have high hopes whenever I have the time to work with Rust but in the short- or even mid-term it is unrealistic to expect that the Rust community will be big enough so that we would find contributors for a niche project like mruby. Also a project which tries to target as many platforms as possible has no other choice than C today. Enough tears shed, here is a list of files which were affected by the audit with an indicator (commits and line changes) to identify were the biggest issues were located:

File Commits Line Changes
src/vm.c 38 324
mrbgems/mruby-compiler/core/codegen.c 28 192
src/array.c 13 144
src/string.c 13 62
test/t/codegen.rb 13 146
src/error.c 11 106
src/kernel.c 8 75
mrbgems/mruby-sprintf/src/sprintf.c 8 103
src/class.c 7 39
src/gc.c 6 48
mrbgems/mruby-struct/test/struct.rb 5 45
src/backtrace.c 5 26
mrbgems/mruby-compiler/core/parse.y 5 12
test/t/proc.rb 4 66
mrbgems/mruby-struct/src/struct.c 4 56
src/proc.c 4 34
test/t/nomethoderror.rb 3 53
mrbgems/mruby-string-ext/mrblib/string.rb 3 61
test/t/kernel.rb 3 26
include/mruby.h 3 47
mrbgems/mruby-string-ext/src/string.c 3 134
mrbgems/mruby-random/src/random.c 2 6
mrbgems/mruby-string-ext/test/string.rb 2 32
test/t/string.rb 2 18
mrbgems/mruby-proc-ext/test/proc.rb 2 22
mrbgems/mruby-proc-ext/src/proc.c 2 8
mrbgems/mruby-time/src/time.c 2 13
mrbgems/mruby-time/test/time.rb 2 22
include/mruby/opcode.h 2 5
src/hash.c 2 23
src/range.c 2 42
test/t/class.rb 1 9
include/mruby/range.h 1 3
mrbgems/mruby-range-ext/src/range.c 1 6
mrbgems/mruby-bin-mruby/bintest/mruby.rb 1 14
mrbgems/mruby-object-ext/test/object.rb 1 28
src/object.c 1 2
include/mruby/irep.h 1 1
src/state.c 1 5
mrbgems/mruby-sprintf/test/sprintf.rb 1 23
include/mruby/boxing_word.h 1 6
mrbgems/mruby-random/test/random.rb 1 12
mrbgems/mruby-compiler/bintest/mrbc.rb 1 9
src/codedump.c 1 18
mrblib/range.rb 1 11
mrbgems/mruby-object-ext/src/object.c 1 6
test/t/exception.rb 1 2

 

Of all code changes 76% were fixes and 24% were regression tests. Ignoring the test code (which is always Ruby) we have a ratio of 94% affected C code and 4% affected Ruby code. Furthermore 58% of all fixes were related to the standard library and 37% to the VM, Parser and Compiler. From that we could conclude that it makes sense to implement even more of the standard library in Ruby itself, if it would not have a to big impact to the size and performance of mruby itself.

Fazit

Personally I like to thank @bouk and @clayton-shopify who done a tremendous job of bringing the bug reports from the bounty to our issue tracker and also wrote many pull-requests even by themselves to fix the issues. Furthermore it doesn’t matter how you think about Shopify and their recent political activities (as I already got some comments concerning this topic), it is an impressive act that they not only publish their mruby-engine (which they don’t need todo because of mruby’s license) but also to keep the bug bounty open and help us to fix bugs which might not even affect their own infrastructure anymore. Last but not least we have to look closely on ourself. The bug bounty is still ongoing and new bugs are discovered. Even though that the reported bug amount is getting lower, this shouldn’t indicate that we are free of issues any time soon or that we have finished the topic of security and stability. It will be a permanent challenge to keep mruby stable and secure.


available also via rss