Page MenuHomePhabricator

Upgrading: `dot` (Graphviz) support removed, changes to `figlet` and `cowsay`
Closed, ResolvedPublic

Tokens
"Heartbreak" token, awarded by mpadourek."Heartbreak" token, awarded by cspeckmim."Heartbreak" token, awarded by tycho.tatitscheff."Heartbreak" token, awarded by cburroughs.
Assigned To
Authored By
epriestley, Sep 13 2015

Description

Summary

The dot, figlet and cowsay remarkup rules are implemented in a way that creates a security risk, and a low-severity (but highly practical) attack has been developed against dot.

The figlet and cowsay rules have been reimplemented safely.

The dot rule can not easily be reimplemented safely and has been removed.

Installs are encouraged to upgrade Phabricator or uninstall dot (Graphviz). Uninstalling dot will defuse the known attack. Upgrading Phabricator will defuse this class of attack.

Installs that rely on the dot rule may install it as an extension. This does not mitigate or defuse the risks. If you do this, you are making your install vulnerable.

Installs with custom Figlet fonts or cows may need to adjust how they are installed (see below).

Details

Phabricator currently ships with three "interpreter" rules in Remarkup: dot (Graphviz), figlet, and cowsay. These rules are invoked like this:

cowsay {{{
Moo!
}}}

These rules are implemented by executing arbitrary binaries on the system. This approach is inherently risky, because executing unsandboxed binaries exposes a huge amount of surface area to attackers. The rules are as safe as possible, given the approach: they are careful about argument handling, the binaries normally need to be explicitly installed by an administrator, and these binaries seemed unlikely to permit arbitrary code execution. But this approach still harbors substantial risk.

A security researcher recently found a practical attack against the dot interpreter which allows an attacker to disclose information about a system and potentially render images on the system into graphs. Although this attack is not especially severe, there is no way to prevent it or other similar attacks (which might be far more severe) under the "execute arbitrary unsandboxed binaries" approach these rules currently employ. You can read the details of the report here once it is disclosed:

https://hackerone.com/reports/88395

In response, we are removing all rules of this type from the upstream: empirically, the risks presented by this approach are too great. The figlet and cowsay rules are simple parsers and could be safely rewritten, but the dot rule is complex. We do not have a safe alternative to the dot rule at this time, and do not have immediate plans to provide one.

Upgrading: Figlet

The figlet rule has been rewritten to only execute trusted code. It is now available on all systems without requiring the figlet binary to be installed.

If you have custom .flf fonts, drop them into phabricator/resources/figlet/custom/ to make them available.

Upgrading: Cowsay

The cowsay rule has been rewritten to only execute trusted code. It is now available on all systems without requiring the cowsay binary to be installed.

If you have custom .cow cows, drop them into phabricator/resources/cows/custom/ to make them available.

Upgrading: dot/Graphviz

This rule has been removed, because we can not easily rewrite it to execute only trusted code or otherwise make it safe.

If you rely on this rule, you may install it as an extension by dropping this file into phabricator/src/extensions/: P1853

IMPORTANT: This extension does not defuse or mitigate the vulnerability this rule creates. Running this extension makes your install vulnerable to attack. Installs are strongly discouraged from running this code.

Event Timeline

epriestley raised the priority of this task from to Normal.
epriestley updated the task description. (Show Details)
epriestley added a subscriber: epriestley.
epriestley updated the task description. (Show Details)Sep 13 2015, 7:30 PM
cburroughs added a subscriber: cburroughs.
_________ < Moooooo > --------- \ ^__^ \ (oo)\_______ (__)\ )\/\ ||----w | || ||

Does the new PHP cowsay implementation need to convert the escaped backslashes to single backslashes?

Ah, I think you're right. I thought that cow just had a nice thick neck (a sign of good breeding), but her tail is a little bristly.

@epriestley - I'm not familiar with how email should work (or how it used to) but I think figlet text (and probably cowsays) are not html-ized for html emails -- they might look better in fixed-width fontstyle but it looks like regular-type. Adding figlet text for test:

____ ____________________________ ____ ________ __ ____ / __ \/ ____/ ____/ ____/ ___/ ___/ / __ )/ ____/ / / / / / / / /_/ / __/ / / / __/ \__ \\__ \ / __ / __/ / / / / / / / / _, _/ /___/ /___/ /___ ___/ /__/ / / /_/ / /___/ /___/ /___/_/_/ /_/ |_/_____/\____/_____//____/____/ /_____/_____/_____/_____(_)_)

Yeah -- at least in HTML mail, the cows aren't getting properly linebreak'd either. I don't think that's new but it should be pretty easy to fix.

epriestley closed this task as Resolved.Oct 18 2015, 3:20 PM
epriestley claimed this task.

There are a couple of minor followups between this original announcement and HEAD which improve some behaviors, but this has been in stable for about a month now.

In https://hackerone.com/reports/259246 (not currently disclosed) a researcher found an actual issue with figlet. Although it would probably be hard to develop into a practical attack, it does make me feel better about the decision to pull all this stuff into PHP (not just dot) when the dot issue was originally identified.