Commit 2e80c7db authored by Hugh Davenport's avatar Hugh Davenport

Fix saved file permissions

Bug #1057238
CVE-2012-2244

Currently, files that are saved by Mahara use the
directorypermissions config option, which defaults to
0700, which allows execution.

This allows users to potentially upload files with
executable bits set, and if they have control of the
config options pathtoclam, pathtozip, or pathtounzip
then they could run this command when one of those
commands are invocated.

This patch bitwise-AND's the directory permissions
config with 0666, which removes any executable bit
and sets the result as a new config option
filepermissions.

A change the upload code to use this new option is made

Change-Id: I088d9873de7797d5a9aefc2401301f8b855ed592
Signed-off-by: default avatarHugh Davenport <hugh@catalyst.net.nz>
parent 4736778a
......@@ -940,6 +940,7 @@ class ArtefactTypeFile extends ArtefactTypeFileBase {
$f->delete();
return false;
}
chmod($newname, get_config('filepermissions'));
$owner = null;
if ($user) {
$owner = $user;
......@@ -2256,7 +2257,7 @@ class ArtefactTypeArchive extends ArtefactTypeFile {
// Untar everything into a temp directory first
$tempsubdir = tempnam($tempdir, '');
unlink($tempsubdir);
mkdir($tempsubdir);
mkdir($tempsubdir, get_config('directorypermissions'));
if (!$this->handle->extract($tempsubdir)) {
throw new SystemException("Unable to extract archive into $tempsubdir");
}
......
......@@ -80,6 +80,7 @@ $CFG->xmldbdisablecommentchecking = true;
if (empty($CFG->directorypermissions)) {
$CFG->directorypermissions = 0700;
}
$CFG->filepermissions = $CFG->directorypermissions & 0666;
// core libraries
require('mahara.php');
......
......@@ -638,7 +638,7 @@ function xmldb_core_upgrade($oldversion=0) {
if (is_dir($artefactdata . 'internal/profileicons')) {
if (!is_dir($artefactdata . 'file')) {
mkdir($artefactdata . 'file');
mkdir($artefactdata . 'file', get_config('directorypermissions'));
}
if (!rename($artefactdata . 'internal/profileicons', $artefactdata . 'file/profileicons')) {
throw new SystemException("Failed moving $artefactdata/internal/profileicons to $artefactdata/file/profileicons");
......
......@@ -795,7 +795,7 @@ function copyr($source, $dest)
// Make destination directory
if (!is_dir($dest)) {
mkdir($dest);
mkdir($dest, get_config('directorypermissions'));
}
// Loop through the folder
......
......@@ -171,7 +171,7 @@ class upload_manager {
$tmpname = $this->file['tmp_name'];
}
if (move_uploaded_file($tmpname, $destination . '/' . $newname)) {
chmod($destination . '/' . $newname, get_config('directorypermissions'));
chmod($destination . '/' . $newname, get_config('filepermissions'));
return false;
}
return get_string('failedmovingfiletodataroot');
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment