From 10e006bdbe7a0d0ff37e99412295eb47538ea929 Mon Sep 17 00:00:00 2001 From: Benjamin Sonntag Date: Tue, 17 May 2016 18:12:57 +0200 Subject: [PATCH 1/4] [security] fixing DO_ACTIONS.PHP for REALPATH. --- src/do_actions.php | 76 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 70 insertions(+), 6 deletions(-) diff --git a/src/do_actions.php b/src/do_actions.php index b42bd580..e9fe7d0f 100644 --- a/src/do_actions.php +++ b/src/do_actions.php @@ -119,6 +119,24 @@ function execute_cmd($command, $parameters=array()) { return array('executed' => $cmd_line, 'output'=>$output, 'return_var'=>$code); } +/** Check if a file or folder is in the list of allowed + * path (after dereferencing all ../ and symlinks + * @param $path string the path to check against + * @return string the dereferenced path, or FALSE if the path is NOT allowed (/var/www/alternc /var/mail/alternc) + */ +function my_realpath($path) { + global $L_ALTERNC_HTML, $L_ALTERNC_MAIL; + // add here any allowed path: + $allowed=array(realpath($L_ALTERNC_HTML)."/", realpath($L_ALTERNC_MAIL)."/"); + $path=realpath($path); + foreach($allowed as $one) { + // the path must be BELOW each allowed folder. forbid anything + if (strlen($path)>strlen($one) && substr($path,0,strlen($one))==$one) { + return $path; + } + } + return false; +} // Check if script isn't already running if (file_exists(ALTERNC_DO_ACTION_LOCK) !== false){ @@ -197,7 +215,11 @@ while ($rr=$action->get_action()){ $returned = execute_cmd("$FIXPERM -u", $params["uid"]); break; case "CHMOD" : - $filename=$params["filename"]; + $filename=my_realpath($params["filename"]); + if ($filename===false) { + $errorsList=array("Fail: path not allowed"); + break; + } $perms=$params["perms"]; // Checks the file or directory exists if( !is_dir($filename) && ! is_file($filename)){ @@ -214,6 +236,13 @@ while ($rr=$action->get_action()){ break; case "CREATE_FILE" : + $dirname=my_realpath(dirname($params["filename"])); + $filename=basename($params["filename"]); + if ($dirname===false) { + $errorsList=array("Fail: path not allowed"); + break; + } + $params["file"]=$dirname.DIRECTORY_SEPARATOR.$filename; if(!file_exists($params["file"])) { if ( file_put_contents($params["file"], $params["content"]) === false ) { $errorsList=array("Fail: can't write into file ".$params["file"]); @@ -227,16 +256,41 @@ while ($rr=$action->get_action()){ } break; case "CREATE_DIR" : - // Create the directory and make parent directories as needed - $returned = execute_cmd("$SU mkdir", array('-p', $params["dir"])); + $dirname=my_realpath(dirname($params["dir"])); + $filename=basename($params["dir"]); + if ($dirname===false) { + $errorsList=array("Fail: path not allowed"); + break; + } + $params["dir"]=$dirname.DIRECTORY_SEPARATOR.$filename; + // Create the directory and make parent directories as needed + $returned = execute_cmd("$SU mkdir", array('-p', $params["dir"])); break; case "DELETE" : - // Delete file/directory and its contents recursively - $returned = execute_cmd("$SU rm", array('-rf', $params["dir"])); + $dirname=my_realpath($params["dir"]); + if ($dirname===false) { + $errorsList=array("Fail: path not allowed"); + break; + } + // Delete file/directory and its contents recursively + $returned = execute_cmd("$SU rm", array('-rf', $dirname)); break; case "MOVE" : // If destination dir does not exists, create it - if(!is_dir($params["dst"])) + $dirname=my_realpath(dirname($params["dst"])); + $filename=basename($params["dst"]); + if ($dirname===false) { + $errorsList=array("Fail: path not allowed"); + break; + } + $params["dst"]=$dirname.DIRECTORY_SEPARATOR.$filename; + $params["src"]=my_realpath($params["src"]); + if ($params["src"]===false) { + $errorsList=array("Fail: path not allowed"); + break; + } + + if (!is_dir($params["dst"])) if ( @mkdir($params["dst"], 0777, true)) { if ( @chown($params["dst"], $r["user"]) ) { $returned = execute_cmd("$SU mv -f", array($params["src"], $params["dst"])); @@ -247,12 +301,22 @@ while ($rr=$action->get_action()){ break; case "FIX_DIR" : + $params["dir"]=my_realpath($params["dir"]); + if ($params["dir"]===false) { + $errorsList=array("Fail: path not allowed"); + break; + } $returned = execute_cmd($FIXPERM, array('-d', $params["dir"])); if($returned['return_val'] != 0) { $errorsList=array("Fixperms.sh failed, returned error code : ".$returned['return_val']); } break; case "FIX_FILE" : + $params["file"]=my_realpath($params["file"]); + if ($params["file"]===false) { + $errorsList=array("Fail: path not allowed"); + break; + } $returned = execute_cmd($FIXPERM, array('-f', $params["file"])); if($returned['return_val'] != 0){ $errorsList=array("Fixperms.sh failed, returned error code : ".$returned['return_val']); From 369ab3bf34c2f80fb1756ff97ecb517ef6ba51c2 Mon Sep 17 00:00:00 2001 From: Benjamin Sonntag Date: Tue, 17 May 2016 18:44:21 +0200 Subject: [PATCH 2/4] [security] using prepared query for scripts too --- src/mail_add.php | 2 +- src/spoolsize.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mail_add.php b/src/mail_add.php index 47ff2b9f..b77a5af2 100644 --- a/src/mail_add.php +++ b/src/mail_add.php @@ -52,7 +52,7 @@ $recipients = array_slice($argv, 2); // rest is recipients // there's no function to do that, oddly enough... // there's one to extract the compte from the mail_id (!) but we // haven't created it yet... -$db->query('SELECT id,compte FROM domaines WHERE domaine="'.addslashes($domain).'"'); +$db->query('SELECT id,compte FROM domaines WHERE domaine=?',array($domain)); if ($db->next_record()) { $compte = $db->f('compte'); $domain_id = $db->f('id'); diff --git a/src/spoolsize.php b/src/spoolsize.php index c6084e28..948cd917 100644 --- a/src/spoolsize.php +++ b/src/spoolsize.php @@ -18,7 +18,7 @@ if ($db->query("SELECT uid,login FROM membres;")) { while ($db->next_record()) { if (isset($list_quota[$db->f('uid')])) { $qu=$list_quota[$db->f('uid')]; - $db2->query("INSERT OR REPLACE INTO size_web SET uid='".intval($db->f('uid'))."',size='".intval($qu['used'])."';"); + $db2->query("INSERT OR REPLACE INTO size_web SET uid=?, size=?;",array(intval($db->f('uid')),intval($qu['used']))); echo $db->f('login')." (".$qu['used']." B)\n"; } } @@ -32,7 +32,7 @@ echo "\n---------------------------\n Generating size-cache for MySQL databases\ $tab=$mysql->get_dbus_size($c["name"],$c["host"],$c["login"],$c["password"],$c["client"]); echo "++ Processing ".$c["name"]." ++\n"; foreach ($tab as $dbname=>$size) { - $db->query("REPLACE INTO size_db SET db='".$dbname."',size='$size';"); + $db->query("REPLACE INTO size_db SET db=?,size=?;",array($dbname,$size)); echo " $dbname done ($size B) \n"; flush(); } echo "\n"; @@ -52,7 +52,7 @@ if ($db->query("SELECT uid, name FROM mailman;")) { $size2=exec("sudo /usr/lib/alternc/du.pl ".escapeshellarg("/var/lib/mailman/archives/private/".$c["name"])); $size3=exec("sudo /usr/lib/alternc/du.pl ".escapeshellarg("/var/lib/mailman/archives/private/".$c["name"].".mbox")); $size=(intval($size1)+intval($size2)+intval($size3)); - $db->query("REPLACE INTO size_mailman SET uid='".$c["uid"]."',list='".$c["name"]."', size='$size';"); + $db->query("REPLACE INTO size_mailman SET uid=?,list=?,size=?;",array($c["uid"],$c["name"],$size)); echo " done ($size KB) \n"; flush(); } } From 0c505e8b6cae6dc8ee73369155961488f225fa01 Mon Sep 17 00:00:00 2001 From: Benjamin Sonntag Date: Tue, 17 May 2016 18:47:09 +0200 Subject: [PATCH 3/4] [security] using prepared query in the panel --- bureau/admin/piwik_useradmin.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bureau/admin/piwik_useradmin.php b/bureau/admin/piwik_useradmin.php index 46c1e6d8..2179c5cc 100644 --- a/bureau/admin/piwik_useradmin.php +++ b/bureau/admin/piwik_useradmin.php @@ -45,7 +45,7 @@ else { // Add a user to a piwik website if ($site_id != -1 && $right !== FALSE) { - $db->query("SELECT COUNT(*) AS ok FROM piwik_sites WHERE uid='$cuid' AND piwik_id='$site_id'"); + $db->query("SELECT COUNT(*) AS ok FROM piwik_sites WHERE uid=? AND piwik_id=?;",array($cuid,$site_id)); $db->next_record(); if ($db->f('ok')!=1) { @@ -53,7 +53,7 @@ else } else { - $db->query("SELECT COUNT(*) AS ok FROM piwik_users WHERE uid='$cuid' AND login='$user_name'"); + $db->query("SELECT COUNT(*) AS ok FROM piwik_users WHERE uid=? AND login=?",array($cuid,$user_name)); $db->next_record(); if ($db->f('ok')!=1) { @@ -79,7 +79,7 @@ else } $user_piwik_sites = array(); - $db->query("SELECT piwik_id FROM piwik_sites WHERE uid='$cuid'"); + $db->query("SELECT piwik_id FROM piwik_sites WHERE uid=?",array($cuid)); while ($db->next_record()) array_push($user_piwik_sites, $db->f('piwik_id')); // Weird behaviour of php: array_push products an array such as: @@ -89,7 +89,7 @@ else $user_piwik_sites = array_flip($user_piwik_sites); $user_piwik_users = array(); - $db->query("SELECT login FROM piwik_users WHERE uid='$cuid'"); + $db->query("SELECT login FROM piwik_users WHERE uid=?",arary($cuid)); while ($db->next_record()) array_push ($user_piwik_users, $db->f('login')); // Swap keys and values, see user_piwik_sites From 9315fbdbaca4830263243d74d0f9a33e728bb608 Mon Sep 17 00:00:00 2001 From: Benjamin Sonntag Date: Tue, 17 May 2016 18:49:34 +0200 Subject: [PATCH 4/4] API too is using PDO, including DB_System --- api/panel/bootstrap.php | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/api/panel/bootstrap.php b/api/panel/bootstrap.php index c135024f..4fe55c98 100644 --- a/api/panel/bootstrap.php +++ b/api/panel/bootstrap.php @@ -25,20 +25,12 @@ require_once($root."class/functions.php"); global $L_MYSQL_HOST,$L_MYSQL_DATABASE,$L_MYSQL_LOGIN,$L_MYSQL_PWD,$db,$dbh; -class DB_system extends DB_Sql { - var $Host,$Database,$User,$Password; - - /** - * Creator - */ - function DB_system() { - global $L_MYSQL_HOST,$L_MYSQL_DATABASE,$L_MYSQL_LOGIN,$L_MYSQL_PWD; - $this->Host = $L_MYSQL_HOST; - $this->Database = $L_MYSQL_DATABASE; - $this->User = $L_MYSQL_LOGIN; - $this->Password = $L_MYSQL_PWD; - } -} +class DB_system extends DB_Sql { + function __construct() { + global $L_MYSQL_HOST,$L_MYSQL_DATABASE,$L_MYSQL_LOGIN,$L_MYSQL_PWD; + parent::__construct($L_MYSQL_DATABASE, $L_MYSQL_HOST, $L_MYSQL_LOGIN, $L_MYSQL_PWD); + } +} // we do both: $db= new DB_system();