Merge pull request #6956 from MrPetovan/task/sanitize-path-item

Sanitize theme/addon path items

mod/settings.php

@@ -30,6 +30,8 @@ use Friendica\Util\Temporal;
 
 function get_theme_config_file($theme)
 {
+	$theme = Strings::sanitizeFilePathItem($theme);
+
 	$a = \get_app();
 	$base_theme = defaults($a->theme_info, 'extends');
 
@@ -877,40 +879,30 @@ function settings_content(App $a)
 			$default_mobile_theme = 'none';
 		}
 
-		$allowed_themes_str = Config::get('system', 'allowed_themes');
+		$allowed_themes = Theme::getAllowedList();
-		$allowed_themes_raw = explode(',', $allowed_themes_str);
-		$allowed_themes = [];
-		if (count($allowed_themes_raw)) {
-			foreach ($allowed_themes_raw as $x) {
-				if (strlen(trim($x)) && is_dir("view/theme/$x")) {
-					$allowed_themes[] = trim($x);
-				}
-			}
-		}
-
 
 		$themes = [];
 		$mobile_themes = ["---" => L10n::t('No special theme for mobile devices')];
-		if ($allowed_themes) {
+		foreach ($allowed_themes as $theme) {
-			foreach ($allowed_themes as $theme) {
+			$is_experimental = file_exists('view/theme/' . $theme . '/experimental');
-				$is_experimental = file_exists('view/theme/' . $theme . '/experimental');
+			$is_unsupported  = file_exists('view/theme/' . $theme . '/unsupported');
-				$is_unsupported  = file_exists('view/theme/' . $theme . '/unsupported');
+			$is_mobile       = file_exists('view/theme/' . $theme . '/mobile');
-				$is_mobile       = file_exists('view/theme/' . $theme . '/mobile');
+			if (!$is_experimental || ($is_experimental && (Config::get('experimentals', 'exp_themes')==1 || is_null(Config::get('experimentals', 'exp_themes'))))) {
-				if (!$is_experimental || ($is_experimental && (Config::get('experimentals', 'exp_themes')==1 || is_null(Config::get('experimentals', 'exp_themes'))))) {
+				$theme_name = ucfirst($theme);
-					$theme_name = ucfirst($theme);
+				if ($is_unsupported) {
-					if ($is_unsupported) {
+					$theme_name = L10n::t('%s - (Unsupported)', $theme_name);
-						$theme_name = L10n::t("%s - \x28Unsupported\x29", $theme_name);
+				} elseif ($is_experimental) {
-					} elseif ($is_experimental) {
+					$theme_name = L10n::t('%s - (Experimental)', $theme_name);
-						$theme_name = L10n::t("%s - \x28Experimental\x29", $theme_name);
+				}
-					}
+
-					if ($is_mobile) {
+				if ($is_mobile) {
-						$mobile_themes[$theme] = $theme_name;
+					$mobile_themes[$theme] = $theme_name;
-					} else {
+				} else {
-						$themes[$theme] = $theme_name;
+					$themes[$theme] = $theme_name;
-					}
 				}
 			}
 		}
+
 		$theme_selected        = defaults($_SESSION, 'theme'       , $default_theme);
 		$mobile_theme_selected = defaults($_SESSION, 'mobile-theme', $default_mobile_theme);
 

mod/view.php

@@ -1,6 +1,7 @@
 <?php
 
 use Friendica\App;
+use Friendica\Util\Strings;
 
 /**
  * load view/theme/$current_theme/style.php with friendica context
@@ -10,14 +11,17 @@ use Friendica\App;
 function view_init(App $a)
 {
 	header("Content-Type: text/css");
-		
+
-	if ($a->argc == 4){
+	if ($a->argc == 4) {
 		$theme = $a->argv[2];
+		$theme = Strings::sanitizeFilePathItem($theme);
+
 		// set the path for later use in the theme styles
 		$THEMEPATH = "view/theme/$theme";
-		if(file_exists("view/theme/$theme/style.php"))
+		if (file_exists("view/theme/$theme/style.php")) {
 			require_once("view/theme/$theme/style.php");
+		}
 	}
-	
+
 	exit();
 }

src/App.php

@@ -10,12 +10,14 @@ use DOMXPath;
 use Exception;
 use Friendica\Core\Config\Cache\IConfigCache;
 use Friendica\Core\Config\Configuration;
+use Friendica\Core\Theme;
 use Friendica\Database\DBA;
 use Friendica\Model\Profile;
 use Friendica\Network\HTTPException\InternalServerErrorException;
 use Friendica\Util\Config\ConfigFileLoader;
 use Friendica\Util\HTTPSignature;
 use Friendica\Util\Profiler;
+use Friendica\Util\Strings;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -975,8 +977,6 @@ class App
 		// Sane default
 		$this->currentTheme = $system_theme;
 
-		$allowed_themes = explode(',', $this->config->get('system', 'allowed_themes', $system_theme));
-
 		$page_theme = null;
 		// Find the theme that belongs to the user whose stuff we are looking at
 		if ($this->profile_uid && ($this->profile_uid != local_user())) {
@@ -1007,8 +1007,9 @@ class App
 			$theme_name = $user_theme;
 		}
 
+		$theme_name = Strings::sanitizeFilePathItem($theme_name);
 		if ($theme_name
-			&& in_array($theme_name, $allowed_themes)
+			&& in_array($theme_name, Theme::getAllowedList())
 			&& (file_exists('view/theme/' . $theme_name . '/style.css')
 			|| file_exists('view/theme/' . $theme_name . '/style.php'))
 		) {

src/Core/Addon.php

@@ -6,6 +6,7 @@ namespace Friendica\Core;
 
 use Friendica\BaseObject;
 use Friendica\Database\DBA;
+use Friendica\Util\Strings;
 
 /**
  * Some functions to handle addons
@@ -81,6 +82,8 @@ class Addon extends BaseObject
 	 */
 	public static function uninstall($addon)
 	{
+		$addon = Strings::sanitizeFilePathItem($addon);
+
 		Logger::notice("Addon {addon}: {action}", ['action' => 'uninstall', 'addon' => $addon]);
 		DBA::delete('addon', ['name' => $addon]);
 
@@ -102,11 +105,13 @@ class Addon extends BaseObject
 	 */
 	public static function install($addon)
 	{
-		// silently fail if addon was removed
+		$addon = Strings::sanitizeFilePathItem($addon);
 
+		// silently fail if addon was removed of if $addon is funky
 		if (!file_exists('addon/' . $addon . '/' . $addon . '.php')) {
 			return false;
 		}
+
 		Logger::notice("Addon {addon}: {action}", ['action' => 'install', 'addon' => $addon]);
 		$t = @filemtime('addon/' . $addon . '/' . $addon . '.php');
 		@include_once('addon/' . $addon . '/' . $addon . '.php');
@@ -130,6 +135,7 @@ class Addon extends BaseObject
 			if (!self::isEnabled($addon)) {
 				self::$addons[] = $addon;
 			}
+
 			return true;
 		} else {
 			Logger::error("Addon {addon}: {action} failed", ['action' => 'uninstall', 'addon' => $addon]);
@@ -153,29 +159,26 @@ class Addon extends BaseObject
 
 			$addon_list = explode(',', $addons);
 
-			if (count($addon_list)) {
+			foreach ($addon_list as $addon) {
-				foreach ($addon_list as $addon) {
+				$addon = Strings::sanitizeFilePathItem(trim($addon));
-					$addon = trim($addon);
+				$fname = 'addon/' . $addon . '/' . $addon . '.php';
-					$fname = 'addon/' . $addon . '/' . $addon . '.php';
+				if (file_exists($fname)) {
+					$t = @filemtime($fname);
+					foreach ($installed as $i) {
+						if (($i['name'] == $addon) && ($i['timestamp'] != $t)) {
 
-					if (file_exists($fname)) {
+							Logger::notice("Addon {addon}: {action}", ['action' => 'reload', 'addon' => $i['name']]);
-						$t = @filemtime($fname);
+							@include_once($fname);
-						foreach ($installed as $i) {
-							if (($i['name'] == $addon) && ($i['timestamp'] != $t)) {
 
-								Logger::notice("Addon {addon}: {action}", ['action' => 'reload', 'addon' => $i['name']]);
+							if (function_exists($addon . '_uninstall')) {
-								@include_once($fname);
+								$func = $addon . '_uninstall';
-
+								$func(self::getApp());
-								if (function_exists($addon . '_uninstall')) {
-									$func = $addon . '_uninstall';
-									$func(self::getApp());
-								}
-								if (function_exists($addon . '_install')) {
-									$func = $addon . '_install';
-									$func(self::getApp());
-								}
-								DBA::update('addon', ['timestamp' => $t], ['id' => $i['id']]);
 							}
+							if (function_exists($addon . '_install')) {
+								$func = $addon . '_install';
+								$func(self::getApp());
+							}
+							DBA::update('addon', ['timestamp' => $t], ['id' => $i['id']]);
 						}
 					}
 				}
@@ -204,6 +207,8 @@ class Addon extends BaseObject
 	{
 		$a = self::getApp();
 
+		$addon = Strings::sanitizeFilePathItem($addon);
+
 		$info = [
 			'name' => $addon,
 			'description' => "",

src/Core/Hook.php

@@ -7,6 +7,7 @@ namespace Friendica\Core;
 use Friendica\App;
 use Friendica\BaseObject;
 use Friendica\Database\DBA;
+use Friendica\Util\Strings;
 
 /**
  * Some functions to handle hooks
@@ -215,6 +216,8 @@ class Hook extends BaseObject
 	 */
 	public static function isAddonApp($name)
 	{
+		$name = Strings::sanitizeFilePathItem($name);
+
 		if (array_key_exists('app_menu', self::$hooks)) {
 			foreach (self::$hooks['app_menu'] as $hook) {
 				if ($hook[0] == 'addon/' . $name . '/' . $name . '.php') {

src/Core/L10n.php

@@ -6,6 +6,7 @@ namespace Friendica\Core;
 
 use Friendica\BaseObject;
 use Friendica\Database\DBA;
+use Friendica\Util\Strings;
 
 /**
  * Provide Language, Translation, and Localization functions to the application
@@ -193,6 +194,8 @@ class L10n extends BaseObject
 	 */
 	private static function loadTranslationTable($lang)
 	{
+		$lang = Strings::sanitizeFilePathItem($lang);
+
 		if ($lang === self::$lang) {
 			return;
 		}
@@ -203,7 +206,7 @@ class L10n extends BaseObject
 		// load enabled addons strings
 		$addons = DBA::select('addon', ['name'], ['installed' => true]);
 		while ($p = DBA::fetch($addons)) {
-			$name = $p['name'];
+			$name = Strings::sanitizeFilePathItem($p['name']);
 			if (file_exists("addon/$name/lang/$lang/strings.php")) {
 				include "addon/$name/lang/$lang/strings.php";
 			}

src/Core/Theme.php

@@ -8,6 +8,7 @@ namespace Friendica\Core;
 
 use Friendica\BaseObject;
 use Friendica\Model\Profile;
+use Friendica\Util\Strings;
 
 require_once 'boot.php';
 
@@ -16,6 +17,23 @@ require_once 'boot.php';
  */
 class Theme
 {
+	public static function getAllowedList()
+	{
+		$allowed_themes_str = Config::get('system', 'allowed_themes');
+		$allowed_themes_raw = explode(',', $allowed_themes_str);
+		$allowed_themes = [];
+		if (count($allowed_themes_raw)) {
+			foreach ($allowed_themes_raw as $theme) {
+				$theme = Strings::sanitizeFilePathItem(trim($theme));
+				if (strlen($theme) && is_dir("view/theme/$theme")) {
+					$allowed_themes[] = $theme;
+				}
+			}
+		}
+
+		return $allowed_themes;
+	}
+
 	/**
 	 * @brief Parse theme comment in search of theme infos.
 	 *
@@ -33,6 +51,8 @@ class Theme
 	 */
 	public static function getInfo($theme)
 	{
+		$theme = Strings::sanitizeFilePathItem($theme);
+
 		$info = [
 			'name' => $theme,
 			'description' => "",
@@ -96,31 +116,37 @@ class Theme
 	 */
 	public static function getScreenshot($theme)
 	{
+		$theme = Strings::sanitizeFilePathItem($theme);
+
 		$exts = ['.png', '.jpg'];
 		foreach ($exts as $ext) {
 			if (file_exists('view/theme/' . $theme . '/screenshot' . $ext)) {
-				return(System::baseUrl() . '/view/theme/' . $theme . '/screenshot' . $ext);
+				return System::baseUrl() . '/view/theme/' . $theme . '/screenshot' . $ext;
 			}
 		}
-		return(System::baseUrl() . '/images/blank.png');
+		return System::baseUrl() . '/images/blank.png';
 	}
 
-	// install and uninstall theme
 	public static function uninstall($theme)
 	{
-		Logger::log("Addons: uninstalling theme " . $theme);
+		$theme = Strings::sanitizeFilePathItem($theme);
 
-		include_once "view/theme/$theme/theme.php";
+		// silently fail if theme was removed or if $theme is funky
-		if (function_exists("{$theme}_uninstall")) {
+		if (file_exists("view/theme/$theme/theme.php")) {
-			$func = "{$theme}_uninstall";
+			Logger::log("Addons: uninstalling theme " . $theme);
-			$func();
+
+			if (function_exists("{$theme}_uninstall")) {
+				$func = "{$theme}_uninstall";
+				$func();
+			}
 		}
 	}
 
 	public static function install($theme)
 	{
-		// silently fail if theme was removed
+		$theme = Strings::sanitizeFilePathItem($theme);
 
+		// silently fail if theme was removed or if $theme is funky
 		if (!file_exists("view/theme/$theme/theme.php")) {
 			return false;
 		}
@@ -166,10 +192,10 @@ class Theme
 			$parent = 'NOPATH';
 		}
 		$theme = \get_app()->getCurrentTheme();
-		$thname = $theme;
+		$parent = Strings::sanitizeFilePathItem($parent);
 		$ext = substr($file, strrpos($file, '.') + 1);
 		$paths = [
-			"{$root}view/theme/$thname/$ext/$file",
+			"{$root}view/theme/$theme/$ext/$file",
 			"{$root}view/theme/$parent/$ext/$file",
 			"{$root}view/$ext/$file",
 		];
@@ -195,6 +221,8 @@ class Theme
 	 */
 	public static function getStylesheetPath($theme)
 	{
+		$theme = Strings::sanitizeFilePathItem($theme);
+
 		if (!file_exists('view/theme/' . $theme . '/style.php')) {
 			return 'view/theme/' . $theme . '/style.css';
 		}

src/Util/Strings.php

@@ -375,4 +375,20 @@ class Strings
   )*
 )@';
 	}
+
+	/**
+	 * Ensures a single path item doesn't contain any path-traversing characters
+	 *
+	 * @see https://stackoverflow.com/a/46097713
+	 * @param string $pathItem
+	 * @return string
+	 */
+	public static function sanitizeFilePathItem($pathItem)
+	{
+		$pathItem = str_replace('/', '_', $pathItem);
+		$pathItem = str_replace('\\', '_', $pathItem);
+		$pathItem = str_replace(DIRECTORY_SEPARATOR, '_', $pathItem); // In case it does not equal the standard values
+
+		return $pathItem;
+	}
 }

view/theme/frio/README.md

@@ -2,7 +2,7 @@
 ### A bootstrap based theme for friendica
 This Theme was started as an experiment to give the user a good looking and modern theme for friendica.
 
-I conentrated on 3 topics:
+I concentrated on 3 topics:
 
 1. A Modern, mobile friendly UI with bootstrap and awesome font
 2. Try to get a new UX for friendica (e.g. use modals where it seems to be useful)

view/theme/frio/php/scheme.php

@@ -19,6 +19,7 @@
  */
 
 use Friendica\Core\PConfig;
+use Friendica\Util\Strings;
 
 function get_scheme_info($scheme)
 {
@@ -28,6 +29,8 @@ function get_scheme_info($scheme)
 		$scheme = PConfig::get(local_user(), 'frio', 'scheme', PConfig::get(local_user(), 'frio', 'schema'));
 	}
 
+	$scheme = Strings::sanitizeFilePathItem($scheme);
+
 	$info = [
 		'name' => $scheme,
 		'description' => '',

view/theme/frio/style.php

@@ -5,9 +5,11 @@
 
 use Friendica\Core\Config;
 use Friendica\Core\PConfig;
+use Friendica\Util\Strings;
 
 require_once 'view/theme/frio/php/PHPColors/Color.php';
 
+$scheme = '';
 $schemecss = '';
 $schemecssfile = false;
 $scheme_modified = 0;
@@ -67,9 +69,7 @@ if (!empty($_REQUEST['scheme'])) {
 	$scheme = $_REQUEST['scheme'];
 }
 
-// Sanitize the data.
+$scheme = Strings::sanitizeFilePathItem($scheme);
-$scheme = !empty($scheme) ? basename($scheme) : '';
-
 
 if (($scheme) && ($scheme != '---')) {
 	if (file_exists('view/theme/frio/scheme/' . $scheme . '.php')) {

view/theme/quattro/style.php

@@ -26,6 +26,8 @@ if ($quattro_align === false) {
 	$quattro_align = $site_quattro_align;
 }
 
+$color = \Friendica\Util\Strings::sanitizeFilePathItem($color);
+
 if (file_exists("$THEMEPATH/$color/style.css")) {
 	echo file_get_contents("$THEMEPATH/$color/style.css");
 }

view/theme/vier/style.php

@@ -22,6 +22,8 @@ if (empty($style)) {
 $stylecss = '';
 $modified = '';
 
+$style = \Friendica\Util\Strings::sanitizeFilePathItem($style);
+
 foreach (['style', $style] as $file) {
 	$stylecssfile = $THEMEPATH . DIRECTORY_SEPARATOR . $file .'.css';
 	if (file_exists($stylecssfile)) {