1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-01-02 02:40:58 +01:00

Prevent Phame blogs from using invalid skins

Summary: Via HackerOne. An attacker with access to both Phame and the filesystem could potentially load a skin that lives outside of the configured skin directories, because we had insufficient checks on the actual skin at load time.

Test Plan: Attempted to build a blog with an invalid skin; got an exception instead of a mis-load of a sketchy skin.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10992
This commit is contained in:
epriestley 2014-12-15 10:41:49 -08:00
parent 2a9db94ba6
commit 2037979142

View file

@ -56,12 +56,32 @@ final class PhameSkinSpecification {
}
public static function loadOneSkinSpecification($name) {
// Only allow skins which we know to exist to load. This prevents loading
// skins like "../../secrets/evil/".
$all = self::loadAllSkinSpecifications();
if (empty($all[$name])) {
throw new Exception(
pht(
'Blog skin "%s" is not a valid skin!',
$name));
}
$paths = PhabricatorEnv::getEnvConfig('phame.skins');
$base = dirname(phutil_get_library_root('phabricator'));
foreach ($paths as $path) {
$path = Filesystem::resolvePath($path, $base);
$skin_path = $path.DIRECTORY_SEPARATOR.$name;
if (is_dir($skin_path)) {
// Double check that the skin really lives in the skin directory.
if (!Filesystem::isDescendant($skin_path, $path)) {
throw new Exception(
pht(
'Blog skin "%s" is not located in path "%s"!',
$name,
$path));
}
$spec = self::loadSkinSpecification($skin_path);
if ($spec) {
$spec->setName($name);
@ -72,7 +92,7 @@ final class PhameSkinSpecification {
return null;
}
public static function loadSkinSpecification($path) {
private static function loadSkinSpecification($path) {
$config_path = $path.DIRECTORY_SEPARATOR.'skin.json';
$config = array();