From 4cbbfdab84c3cd58880902dbc560b14b4c66a0e8 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 2 Sep 2010 10:40:41 -0700 Subject: [PATCH 1/2] Fix for #2635: use ssl-sometimes settings for Twitter settings & auth pages --- lib/util.php | 3 +-- plugins/TwitterBridge/TwitterBridgePlugin.php | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/util.php b/lib/util.php index 66600c766f..f63e152e33 100644 --- a/lib/util.php +++ b/lib/util.php @@ -1018,8 +1018,7 @@ function common_local_url($action, $args=null, $params=null, $fragment=null, $ad function common_is_sensitive($action) { - static $sensitive = array('login', 'register', 'passwordsettings', - 'twittersettings', 'api'); + static $sensitive = array('login', 'register', 'passwordsettings', 'api'); $ssl = null; if (Event::handle('SensitiveAction', array($action, &$ssl))) { diff --git a/plugins/TwitterBridge/TwitterBridgePlugin.php b/plugins/TwitterBridge/TwitterBridgePlugin.php index 0505a328fb..8e3eba3186 100644 --- a/plugins/TwitterBridge/TwitterBridgePlugin.php +++ b/plugins/TwitterBridge/TwitterBridgePlugin.php @@ -335,5 +335,30 @@ class TwitterBridgePlugin extends Plugin return (bool)$this->adminImportControl; } + /** + * When the site is set to ssl=sometimes mode, we should make sure our + * various auth-related pages are on SSL to keep things looking happy. + * Although we're not submitting passwords directly, we do link out to + * an authentication source and it's a lot happier if we've got some + * protection against MitM. + * + * @param string $action name + * @param boolean $ssl outval to force SSL + * @return mixed hook return value + */ + function onSensitiveAction($action, &$ssl) + { + $sensitive = array('twitteradminpanel', + 'twittersettings', + 'twitterauthorization', + 'twitterlogin'); + if (in_array($action, $sensitive)) { + $ssl = true; + return false; + } else { + return true; + } + } + } From 11f7fce3bb59af46dd76c1e219f8df04de9e03af Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 2 Sep 2010 12:11:45 -0700 Subject: [PATCH 2/2] Fixes for custom theme upload: * skip more files (.xcf image sources, .html docs) * skip files before rejecting them for funky filenames! * allow period in filenames (eg foo-1.4.ttf) but blacklist some unsafe extensions-within-extensions --- lib/themeuploader.php | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/lib/themeuploader.php b/lib/themeuploader.php index 370965db08..abf0658d31 100644 --- a/lib/themeuploader.php +++ b/lib/themeuploader.php @@ -128,8 +128,16 @@ class ThemeUploader continue; } - // Check the directory structure... + // Is this a safe or skippable file? $path = pathinfo($name); + if ($this->skippable($path['filename'], $path['extension'])) { + // Documentation and such... booooring + continue; + } else { + $this->validateFile($path['filename'], $path['extension']); + } + + // Check the directory structure... $dirs = explode('/', $path['dirname']); $baseDir = array_shift($dirs); if ($commonBaseDir === false) { @@ -144,14 +152,6 @@ class ThemeUploader $this->validateFileOrFolder($dir); } - // Is this a safe or skippable file? - if ($this->skippable($path['filename'], $path['extension'])) { - // Documentation and such... booooring - continue; - } else { - $this->validateFile($path['filename'], $path['extension']); - } - $fullPath = $dirs; $fullPath[] = $path['basename']; $localFile = implode('/', $fullPath); @@ -180,9 +180,12 @@ class ThemeUploader } } + /** + * @fixme Probably most unrecognized files should just be skipped... + */ protected function skippable($filename, $ext) { - $skip = array('txt', 'rtf', 'doc', 'docx', 'odt'); + $skip = array('txt', 'html', 'rtf', 'doc', 'docx', 'odt', 'xcf'); if (strtolower($filename) == 'readme') { return true; } @@ -201,17 +204,24 @@ class ThemeUploader protected function validateFileOrFolder($name) { - if (!preg_match('/^[a-z0-9_-]+$/i', $name)) { + if (!preg_match('/^[a-z0-9_\.-]+$/i', $name)) { $msg = _("Theme contains invalid file or folder name. " . "Stick with ASCII letters, digits, underscore, and minus sign."); throw new ClientException($msg); } + if (preg_match('/\.(php|cgi|asp|aspx|js|vb)\w/i', $name)) { + $msg = _("Theme contains unsafe file extension names; may be unsafe."); + throw new ClientException($msg); + } return true; } protected function validateExtension($ext) { - $allowed = array('css', 'png', 'gif', 'jpg', 'jpeg'); + $allowed = array('css', // CSS may need validation + 'png', 'gif', 'jpg', 'jpeg', + 'svg', // SVG images/fonts may need validation + 'ttf', 'eot', 'woff'); if (!in_array(strtolower($ext), $allowed)) { $msg = sprintf(_("Theme contains file of type '.%s', " . "which is not allowed."),