Skip to content

Commit

Permalink
Security fixes
Browse files Browse the repository at this point in the history
- Disable unused upload form: /main/upload/upload.document.php.
- Update .htaccess to disable php execution inside web/ (before it was
only web/css).
- Add phar extension in the php2phps() function
- Document upload, check the destination path to be inside the course
with Security::check_abs_path
- Add api_protect_course_script()
- Add course/user validations
  • Loading branch information
jmontoyaa committed Apr 19, 2021
1 parent d41ce4e commit f65d065
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 5 deletions.
26 changes: 22 additions & 4 deletions main/inc/lib/fileUpload.lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
*/
function php2phps($file_name)
{
return preg_replace('/\.(php.?|phtml.?)(\.){0,1}.*$/i', '.phps', $file_name);
return preg_replace('/\.(phar.?|php.?|phtml.?)(\.){0,1}.*$/i', '.phps', $file_name);
}

/**
Expand Down Expand Up @@ -238,7 +238,7 @@ function handle_uploaded_document(
$sessionId = null,
$treat_spaces_as_hyphens = true
) {
if (!$userId) {
if (empty($uploadedFile) || empty($userId) || empty($courseInfo) || empty($documentDir) || empty($uploadPath)) {
return false;
}

Expand All @@ -258,7 +258,6 @@ function handle_uploaded_document(

// Just in case process_uploaded_file is not called
$maxSpace = DocumentManager::get_course_quota();

// Check if there is enough space to save the file
if (!DocumentManager::enough_space($uploadedFile['size'], $maxSpace)) {
if ($output) {
Expand All @@ -268,6 +267,21 @@ function handle_uploaded_document(
return false;
}

if ($uploadPath !== '/') {
$uploadPath = $uploadPath.'/';
}

if (!Security::check_abs_path($documentDir.$uploadPath, $documentDir.'/')) {
Display::addFlash(
Display::return_message(
get_lang('Forbidden'),
'error'
)
);
return false;
}


// If the want to unzip, check if the file has a .zip (or ZIP,Zip,ZiP,...) extension
if ($unzip == 1 && preg_match('/.zip$/', strtolower($uploadedFile['name']))) {
return unzip_uploaded_document(
Expand Down Expand Up @@ -310,7 +324,7 @@ function handle_uploaded_document(
return false;
} else {
// If the upload path differs from / (= root) it will need a slash at the end
if ($uploadPath != '/') {
if ($uploadPath !== '/') {
$uploadPath = $uploadPath.'/';
}

Expand Down Expand Up @@ -1137,6 +1151,10 @@ function unzip_uploaded_document(
$onlyUploadFile = false,
$whatIfFileExists = 'overwrite'
) {
if (empty($courseInfo) || empty($userInfo) || empty($uploaded_file) || empty($uploadPath)) {
return false;
}

$zip = new PclZip($uploaded_file['tmp_name']);

// Check the zip content (real size and file extension)
Expand Down
2 changes: 2 additions & 0 deletions main/upload/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

$_course = api_get_course_info();

api_protect_course_script(true);

$htmlHeadXtra[] = "<script>
function check_unzip() {
if (document.upload.unzip.checked) {
Expand Down
2 changes: 2 additions & 0 deletions main/upload/upload.document.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
<?php
/* For licensing terms, see /license.txt */

exit;

/**
* Process part of the document sub-process for upload. This script MUST BE included by upload/index.php
* as it prepares most of the variables needed here.
Expand Down
4 changes: 3 additions & 1 deletion main/upload/upload.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*/
require_once __DIR__.'/../inc/global.inc.php';

api_protect_course_script(true);

$toolFromSession = Session::read('my_tool');

// return to index if no tool is set
Expand Down Expand Up @@ -40,6 +42,6 @@
case TOOL_STUDENTPUBLICATION:
case TOOL_DOCUMENT:
default:
require 'upload.document.php';
//require 'upload.document.php';
break;
}
2 changes: 2 additions & 0 deletions main/upload/upload_ppt.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*/
require_once __DIR__.'/../inc/global.inc.php';

api_protect_course_script(true);

if (isset($_POST['convert'])) {
$cwdir = getcwd();
if (isset($_FILES['user_file'])) {
Expand Down
2 changes: 2 additions & 0 deletions main/upload/upload_word.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
*/
require_once __DIR__.'/../inc/global.inc.php';

api_protect_course_script(true);

$form_style = '<style>
.row {
width: 200px;
Expand Down

0 comments on commit f65d065

Please sign in to comment.