A Journey to Truly Safe HTML Rendering

We leverage Rubocop’s OutputSafety check to ensure we’re being diligent about safe HTML rendering, so when we found vulnerabilities, we fixed them.

As developers of financial software on the web, one of our biggest responsibilities is to keep our applications secure. One area we need to be conscious of is how we render HTML. If we don’t escape content properly, we could open ourselves and our customers up to security risks. We take this seriously at Betterment, so we use tools like Rubocop, the Ruby static analysis tool, to keep us on the right track. When we found that Rubocop’s OutputSafety check had some holes, we plugged them.

What does it mean to escape content?

Escaping content simply means replacing special characters with entities so that HTML understands to print those characters rather than act upon their special meanings. For example, the < character is escaped using &lt;, the >character is escaped using &gt;, and the & character is escaped using &amp;.

What could happen if we don’t escape content?

We escape content primarily to avoid opening ourselves up to XSS (cross-site scripting) attacks. If we were to inject user-provided content onto a page without escaping it, we’d be vulnerable to executing malicious code in the user’s browser, allowing an attacker full control over a customer’s session.This resource is helpful to learn more about XSS.

Rails makes escaping content easier

Rails escapes content by default in some scenarios, including when tag helpers are used. In addition, Rails has a few methods that provide help in escaping content. safejoin escapes the content and returns a SafeBuffer (a String flagged as safe) containing it. On the other hand, some methods are just a means for us to mark content as already safe. For example, the <%==interpolation token renders content as is and rawhtmlsafe, and safe_concat simply return a SafeBuffer containing the original content as is, which poses a security risk. If content is inside a SafeBuffer, Rails won’t try to escape it upon rendering.

Some examples:


[1] pry(main)> include ActionView::Helpers::OutputSafetyHelper
=> Object
[2] pry(main)> result = “<p>hi</p>”.html_safe
=> “<p>hi</p>”
[3] pry(main)> result.class
=> ActiveSupport::SafeBuffer


[1] pry(main)> result = raw(“<p>hi</p>”)
=> “<p>hi</p>”
[2] pry(main)> result.class
=> ActiveSupport::SafeBuffer


[1] pry(main)> include ActionView::Helpers::TextHelper
=> Object
[2] pry(main)> buffer1 = “<p>hi</p>”.html_safe
=> “<p>hi</p>”
[3] pry(main)> result = buffer1.safe_concat(“<p>bye</p>”)
=> “<p>hi</p><p>bye</p>”
[4] pry(main)> result.class
=> ActiveSupport::SafeBuffer


[1] pry(main)> include ActionView::Helpers::OutputSafetyHelper
=> Object
[2] pry(main)> result = safe_join([“<p>hi</p>”, “<p>bye</p>”])
=> “&lt;p&gt;hi&lt;/p&gt;&lt;p&gt;bye&lt;/p&gt;”
[3] pry(main)> result.class
=> ActiveSupport::SafeBuffer
=> ActiveSupport::SafeBuffer

Rubocop: we’re safe!

As demonstrated, Rails provides some methods that mark content as safe without escaping it for us. Rubocop, a popular Ruby static analysis tool, provides a cop (which is what Rubocop calls a “check”) to alert us when we’re using these methods: Rails/OutputSafety. At Betterment, we explicitly enable this cop in our Rubocop configurations so if a developer wants to mark content as safe, they will need to explicitly disable the cop. This forces extra thought and extra conversation in code review to ensure that the usage is in fact safe.

… Almost

We were thrilled about the introduction of this cop — we had actually written custom cops prior to its introduction to protect us against using the methods that don’t escape content. However, we realized there were some issues with the opinions the cop held about some of these methods.

The first of these issues was that the cop allowed usage of raw and htmlsafewhen the usages were wrapped in safejoin. The problem with this is that when raw or htmlsafe are used to mark content as already safe by putting it in a SafeBuffer as is, safejoin will not actually do anything additional to escape the content. This means that these usages of raw and html_safeshould still be violations.

The second of these issues was that the cop prevented usages of raw and htmlsafe, but did not prevent usages of safeconcatsafeconcat has the same functionality as raw and htmlsafe — it simply marks the content safe as is by returning it in a SafeBuffer. Therefore, the cop should hold the same opinions about safe_concat as it does about the other two methods.

So, we fixed it

Rather than continue to use our custom cops, we decided to give back to the community and fix the issues we had found with the Rails/OutputSafety cop. We began with this pull request to patch the first issue — change the behavior of the cop to recognize raw and htmlsafe as violations regardless of being wrapped in safejoin. We found the Rubocop community to be welcoming, making only minor suggestions before merging our contribution. We followed up shortly after with a pull request to patch the second issue — change the behavior of the cop to disallow usages of safe_concat. This contribution was merged as well.

Contributing to Rubocop was such a nice experience that when we later found that we’d like to add a configuration option to an unrelated cop, we felt great about opening a pull request to do so, which was merged as well.

And here we are!

Our engineering team here at Betterment takes security seriously. We leverage tools like Rubocop and Brakeman, a static analysis tool specifically focused on security, to make our software safe by default against many of the most common security errors, even for code we haven’t written yet. We now rely on Rubocop’s Rails/OutputSafety cop (instead of our custom cop) to help ensure that our team is making good decisions about escaping HTML content. Along the way, we were able to contribute back to a great community.

This article is part of Engineering at Betterment.