Skip to content

Commit

Permalink
Improve local file inclusion protections
Browse files Browse the repository at this point in the history 
The existing protections would not work on Windows platforms with backslash path delimiters. This commit improves the LFI protections throughout the core to remove any sequence of 2 or more `.` characters regardless of the path delimiter that precedes or follows it.
  • Loading branch information
@opengeek
opengeek committed Apr 19, 2017
1 parent 8f137b1 commit e873488
Show file tree
Hide file tree
Showing 19 changed files with 25 additions and 25 deletions.
2 changes: 1 addition & 1 deletion core/model/modx/modconnectorresponse.class.php
View file
Expand Up @@ -90,7 +90,7 @@ public function outputContent(array $options = array()) {
/* backwards compat */
$error =& $this->modx->error;
/* prevent browsing of subdirectories for security */
$target = preg_replace('/(\.+\/)+/', '', htmlspecialchars($options['action']));
$target = preg_replace('/[\.]{2,}/', '', htmlspecialchars($options['action']));

$siteId = $this->modx->user->getUserToken($this->modx->context->get('key'));
$isLogin = $target == 'login' || $target == 'security/login';
Expand Down
2 changes: 1 addition & 1 deletion core/model/modx/modx.class.php
View file
Expand Up @@ -1663,7 +1663,7 @@ public function runProcessor($action = '',$scriptProperties = array(),$options =
if (isset($options['location']) && !empty($options['location'])) $processorsPath .= ltrim($options['location'],'/') . '/';

// Prevent path traversal through the action
$action = preg_replace('/(\.+\/)+/', '', htmlspecialchars($action));
$action = preg_replace('/[\.]{2,}/', '', htmlspecialchars($action));

// Find the processor file, preferring class based processors over old-style processors
$processorFile = $processorsPath.ltrim($action . '.class.php','/');
Expand Down
2 changes: 1 addition & 1 deletion core/model/modx/processors/browser/directory/chmod.class.php
View file
Expand Up @@ -39,7 +39,7 @@ public function process() {
$this->source->initialize();

$dir = $this->getProperty('dir');
$dir = preg_replace('/(\.+\/)+/', '', htmlspecialchars($dir));
$dir = preg_replace('/[\.]{2,}/', '', htmlspecialchars($dir));
$success = $this->source->chmodContainer($dir, $this->getProperty('mode'));

if (empty($success)) {
Expand Down
4 changes: 2 additions & 2 deletions core/model/modx/processors/browser/directory/create.class.php
View file
Expand Up @@ -41,10 +41,10 @@ public function process() {
}

$parent = rawurldecode($this->getProperty('parent',''));
$parent = ltrim(strip_tags(preg_replace('/(\.+\/)+/', '', htmlspecialchars($parent))),'/');
$parent = ltrim(strip_tags(preg_replace('/[\.]{2,}/', '', htmlspecialchars($parent))),'/');

$name = $this->getProperty('name');
$name = ltrim(strip_tags(preg_replace('/(\.+\/)+/', '', htmlspecialchars($name))),'/');
$name = ltrim(strip_tags(preg_replace('/[\.]{2,}/', '', htmlspecialchars($name))),'/');
$success = $this->source->createContainer($name, $parent);

if (empty($success)) {
Expand Down
2 changes: 1 addition & 1 deletion core/model/modx/processors/browser/directory/getfiles.class.php
View file
Expand Up @@ -49,7 +49,7 @@ public function process() {
}

$dir = $this->getProperty('dir');
$dir = preg_replace('/(\.+\/)+/', '', htmlspecialchars($dir));
$dir = preg_replace('/[\.]{2,}/', '', htmlspecialchars($dir));
if ($dir === 'root') {
$dir = '';
}
Expand Down
2 changes: 1 addition & 1 deletion core/model/modx/processors/browser/directory/getlist.class.php
View file
Expand Up @@ -29,7 +29,7 @@ public function initialize() {
'id' => '',
));
$dir = $this->getProperty('id');
$dir = preg_replace('/(\.+\/)+/', '', htmlspecialchars($dir));
$dir = preg_replace('/[\.]{2,}/', '', htmlspecialchars($dir));
if (empty($dir) || $dir === 'root') {
$this->setProperty('id','');
} else if (strpos($dir, 'n_') === 0) {
Expand Down
2 changes: 1 addition & 1 deletion core/model/modx/processors/browser/directory/remove.class.php
View file
Expand Up @@ -41,7 +41,7 @@ public function process() {
}

$dir = $this->getProperty('dir');
$dir = preg_replace('/(\.+\/)+/', '', htmlspecialchars($dir));
$dir = preg_replace('/[\.]{2,}/', '', htmlspecialchars($dir));
$success = $this->source->removeContainer($dir);

if (empty($success)) {
Expand Down
4 changes: 2 additions & 2 deletions core/model/modx/processors/browser/directory/rename.class.php
View file
Expand Up @@ -45,9 +45,9 @@ public function process() {
}

$path = $this->getProperty('path');
$path = preg_replace('/(\.+\/)+/', '', htmlspecialchars($path));
$path = preg_replace('/[\.]{2,}/', '', htmlspecialchars($path));
$name = $this->getProperty('name');
$name = preg_replace('/(\.+\/)+/', '', htmlspecialchars($name));
$name = preg_replace('/[\.]{2,}/', '', htmlspecialchars($name));
$response = $this->source->renameContainer($path, $name);
return $this->handleResponse($response);
}
Expand Down
4 changes: 2 additions & 2 deletions core/model/modx/processors/browser/directory/sort.class.php
View file
Expand Up @@ -18,9 +18,9 @@ public function getLanguageTopics() {
}
public function process() {
$from = $this->getProperty('from');
$from = preg_replace('/(\.+\/)+/', '', htmlspecialchars($from));
$from = preg_replace('/[\.]{2,}/', '', htmlspecialchars($from));
$to = $this->getProperty('to');
$to = preg_replace('/(\.+\/)+/', '', htmlspecialchars($to));
$to = preg_replace('/[\.]{2,}/', '', htmlspecialchars($to));
$point = $this->getProperty('point','append');
if (empty($from)) return $this->failure($this->modx->lexicon('file_folder_err_ns'));
if (empty($to)) return $this->failure($this->modx->lexicon('file_folder_err_ns'));
Expand Down
4 changes: 2 additions & 2 deletions core/model/modx/processors/browser/directory/update.class.php
View file
Expand Up @@ -38,8 +38,8 @@ public function process() {
return $this->failure($this->modx->lexicon('permission_denied'));
}

$dir = preg_replace('/(\.+\/)+/', '', htmlspecialchars($this->getProperty('dir')));
$name = preg_replace('/(\.+\/)+/', '', htmlspecialchars($this->getProperty('name')));
$dir = preg_replace('/[\.]{2,}/', '', htmlspecialchars($this->getProperty('dir')));
$name = preg_replace('/[\.]{2,}/', '', htmlspecialchars($this->getProperty('name')));
$success = $source->renameContainer($dir, $name);

if (!$success) {
Expand Down
4 changes: 2 additions & 2 deletions core/model/modx/processors/browser/file/create.class.php
View file
Expand Up @@ -21,10 +21,10 @@ public function getLanguageTopics() {
public function process() {
/* get base paths and sanitize incoming paths */
$directory = rawurldecode($this->getProperty('directory',''));
$directory = ltrim(strip_tags(preg_replace('/(\.+\/)+/', '', htmlspecialchars($directory))),'/');
$directory = ltrim(strip_tags(preg_replace('/[\.]{2,}/', '', htmlspecialchars($directory))),'/');

$name = $this->getProperty('name');
$name = ltrim(strip_tags(preg_replace('/(\.+\/)+/', '', htmlspecialchars($name))),'/');
$name = ltrim(strip_tags(preg_replace('/[\.]{2,}/', '', htmlspecialchars($name))),'/');

$loaded = $this->getSource();
if (!($this->source instanceof modMediaSource)) {
Expand Down
2 changes: 1 addition & 1 deletion core/model/modx/processors/browser/file/download.class.php
View file
Expand Up @@ -28,7 +28,7 @@ public function process() {
public function getObjectUrl() {
/* format filename */
$file = rawurldecode($this->getProperty('file',''));
$file = preg_replace('/(\.+\/)+/', '', htmlspecialchars($file));
$file = preg_replace('/[\.]{2,}/', '', htmlspecialchars($file));
$url = $this->source->getObjectUrl($file);
return $this->success('',array('url' => $url));
}
Expand Down
2 changes: 1 addition & 1 deletion core/model/modx/processors/browser/file/get.class.php
View file
Expand Up @@ -20,7 +20,7 @@ public function getLanguageTopics() {
public function process() {
/* format filename */
$file = rawurldecode($this->getProperty('file',''));
$file = preg_replace('/(\.+\/)+/', '', htmlspecialchars($file));
$file = preg_replace('/[\.]{2,}/', '', htmlspecialchars($file));

$loaded = $this->getSource();
if ($loaded !== true) {
Expand Down
2 changes: 1 addition & 1 deletion core/model/modx/processors/browser/file/remove.class.php
View file
Expand Up @@ -24,7 +24,7 @@ public function process() {
if (empty($file)) {
return $this->modx->error->failure($this->modx->lexicon('file_err_ns'));
}
$file = preg_replace('/(\.+\/)+/', '', htmlspecialchars($file));
$file = preg_replace('/[\.]{2,}/', '', htmlspecialchars($file));

$loaded = $this->getSource();
if (!($this->source instanceof modMediaSource)) {
Expand Down
4 changes: 2 additions & 2 deletions core/model/modx/processors/browser/file/rename.class.php
View file
Expand Up @@ -31,9 +31,9 @@ public function process() {
}

$oldFile = $this->getProperty('path');
$oldFile = preg_replace('/(\.+\/)+/', '', htmlspecialchars($oldFile));
$oldFile = preg_replace('/[\.]{2,}/', '', htmlspecialchars($oldFile));
$name = $this->getProperty('name');
$name = preg_replace('/(\.+\/)+/', '', htmlspecialchars($name));
$name = preg_replace('/[\.]{2,}/', '', htmlspecialchars($name));
$success = $this->source->renameObject($oldFile, $name);

if (empty($success)) {
Expand Down
2 changes: 1 addition & 1 deletion core/model/modx/processors/browser/file/unpack.class.php
View file
Expand Up @@ -30,7 +30,7 @@ public function process() {
$this->modx->getService('fileHandler', 'modFileHandler');

$target = $this->modx->getOption('base_path') . $this->properties['path'] . $this->properties['file'];
$target = preg_replace('/(\.+\/)+/', '', htmlspecialchars($target));
$target = preg_replace('/[\.]{2,}/', '', htmlspecialchars($target));
$fileobj = $this->modx->fileHandler->make($target);

if (!$this->validate($fileobj)) {
Expand Down
2 changes: 1 addition & 1 deletion core/model/modx/processors/browser/file/update.class.php
View file
Expand Up @@ -21,7 +21,7 @@ public function getLanguageTopics() {
public function process() {
/* get base paths and sanitize incoming paths */
$filePath = rawurldecode($this->getProperty('file',''));
$filePath = preg_replace('/(\.+\/)+/', '', htmlspecialchars($filePath));
$filePath = preg_replace('/[\.]{2,}/', '', htmlspecialchars($filePath));

$loaded = $this->getSource();
if (!($this->source instanceof modMediaSource)) {
Expand Down
2 changes: 1 addition & 1 deletion core/model/modx/processors/browser/file/upload.class.php
View file
Expand Up @@ -37,7 +37,7 @@ public function process() {
return $this->failure($this->modx->lexicon('permission_denied'));
}

$path = preg_replace('/(\.+\/)+/', '', htmlspecialchars($this->getProperty('path')));
$path = preg_replace('/[\.]{2,}/', '', htmlspecialchars($this->getProperty('path')));
$success = $this->source->uploadObjectsToContainer($path,$_FILES);

if (empty($success)) {
Expand Down
2 changes: 1 addition & 1 deletion setup/includes/modinstall.class.php
View file
Expand Up @@ -52,7 +52,7 @@ class modInstall {
*/
function __construct(array $options = array()) {
if (isset ($_REQUEST['action'])) {
$this->action = preg_replace('/(\.+\/)+/', '', htmlspecialchars($_REQUEST['action']));
$this->action = preg_replace('/[\.]{2,}/', '', htmlspecialchars($_REQUEST['action']));
}
if (is_array($options)) {
$this->options = $options;
Expand Down

0 comments on commit e873488

Please sign in to comment.