Plugging holes in ORB

rubysecurityopen-source

ORB had been in use for a few months at this point. We’d shipped it, built Sirius on it, written a lot of templates. It worked. But we’d never done a proper security review of the compilation pipeline, and that had been nagging at me. Template engines compile to code that executes. If there’s a way to inject into that code, you’ve got a problem.

We commissioned a proper security audit: a manual review tracing every path from template source through the tokenizer, parser, compiler, and Temple filters into the generated Ruby. The kind of review that cross-references against historical CVEs in ERB, HAML, and SLIM and asks “are we making the same mistakes?”

The report came back on March 12th with ten real findings. One critical, five high, three medium, one low. Plus a couple of informational items that are inherent to any template engine that executes Ruby (same as ERB: templates are trusted developer code, not user input).

Not what you want to read, but exactly what you need to know.

The critical one

The :for directive used split(' in ') to separate the iterator variable from the collection. That’s it. No validation. Both halves got interpolated directly into a Ruby string:

code = "#{collection}.each do |#{enumerator}|"

Which means you could write:

{#for x in [1]; system("pwned"); [2]}

And the compiler would happily generate [1]; system("pwned"); [2].each do |x|. Arbitrary code execution through a template expression. The enumerator side was injectable too, through the block parameter delimiters.

The fix was a strict regex: the enumerator must be a valid Ruby identifier ([a-z_]\w*), and the collection can’t contain semicolons. Simple, obvious in retrospect, and the kind of thing that should have been there from day one.

The attribute XSS

Dynamic attribute values, things like class={some_expression}, were being emitted as [:dynamic, ...] without an escape wrapper. Printing expressions ({{...}}) were properly escaped, but attributes weren’t getting the same treatment. If the expression evaluated to something containing "><script>alert(1)</script>, the output would be exploitable.

This one is analogous to CVE-2017-1002201 in HAML and CVE-2016-6316 in Rails. The fix was wrapping dynamic attribute values in [:escape, true, ...] so Temple’s Escapable filter runs escape_html on them, same as it does for printing expressions. One line change.

Five ways to inject through names

This was the pattern that kept repeating. ORB interpolates component names, slot names, tag names, and :with directive values into generated Ruby code. The tokenizer’s patterns were permissive. TAG_NAME was [^\s>/=$]+, which allows parentheses, semicolons, single quotes, and basically anything that isn’t whitespace or an angle bracket.

So you could write <Kernel.exit(1)> as a component name, and the compiler would generate render Kernel::exit(1).new(). Kernel::exit(1) executes immediately before .new() is ever reached. Or <Card:Foo();system(1);x> for a slot, which injects system(1) into the generated method call.

Each injection point needed its own validation:

The :with fix was a breaking change: templates using <Foo::bar> style slot syntax stopped working. The canonical syntax <Foo:Bar> was always the intended form, so we shipped the break.

The medium findings

Unbounded brace nesting in the tokenizer: a template with millions of opening braces would eat all available memory. We capped nesting at 100 levels. Generous for any real template, lethal for abuse.

The runtime_error method was interpolating error messages into a %q[...] string literal. An error message containing ] would break out of the delimiter. We switched to String#inspect, which properly escapes everything.

The ATTRIBUTE_NAME regex was too permissive, allowing quotes and backticks in attribute names. Tightened it to match the HTML spec: letters, digits, hyphens, dots, colons.

Forty-eight hours

When the report landed, we dropped everything. A critical code injection in a template engine isn’t something you sit on. In the following two days we spent our entire effort to get all ten findings resolved and pushed: each fix in its own commit (security-fix: CRITICAL-1, security-fix: HIGH-1, etc.), each backed by dedicated regression tests.

Looking back, most of these are obvious. The :for split is embarrassing. You don’t interpolate into generated code without validation, everyone knows this. The attribute escaping gap is a known pattern with CVEs in other engines. The name injection through permissive regexes is textbook.

But that’s the whole point of getting an audit done. Template engines are deceptive. They look simple: parse some markup, generate some code, done. The compilation pipeline has a lot of surface area though, and every place where a template value gets interpolated into generated Ruby is a place where injection can happen. ERB, HAML, and SLIM all had similar CVEs at various points in their history.

Version 0.2.0 shipped with all ten findings resolved and a security test suite of 37 tests covering every finding and its mitigations. We also went back and did a second pass over the full pipeline to make sure we hadn’t missed anything structurally similar. The audit was a wakeup call, but ORB is a better library for it, and we can point to the test suite and say exactly what’s covered.