Skip to content

Commit 333d13a

Browse files
author
Marius Burkard
committed
Merge branch 'develop' into 'develop'
Merge ssh user security enhancement Closes #5920 See merge request ispconfig/ispconfig3!1333
2 parents f058c7e + 9f43b35 commit 333d13a

File tree

7 files changed

+181
-28
lines changed

7 files changed

+181
-28
lines changed

‎interface/lib/classes/functions.inc.php‎

Lines changed: 53 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public function mail($to, $subject, $text, $from, $filepath = '', $filetype = 'a
6161
if(is_string($to) && strpos($to, ',') !== false) {
6262
$to = preg_split('/\s*,\s*/', $to);
6363
}
64-
64+
6565
$app->ispcmail->send($to);
6666
$app->ispcmail->finish();
6767

@@ -234,7 +234,7 @@ public function suggest_ips($type = 'IPv4'){
234234
if(preg_match($regex, $result['ip'])) $ips[] = $result['ip'];
235235
}
236236
}
237-
237+
238238
$results = $app->db->queryAllRecords("SELECT remote_ips FROM web_database WHERE remote_ips != ''");
239239
if(!empty($results) && is_array($results)){
240240
foreach($results as $result){
@@ -290,6 +290,34 @@ public function formatBytes($size, $precision = 2) {
290290
return round(pow(1024, $base-floor($base)), $precision).$suffixes[floor($base)];
291291
}
292292

293+
294+
/**
295+
* Normalize a path and strip duplicate slashes from it
296+
*
297+
* This will also remove all /../ from the path, reducing the preceding path elements
298+
*
299+
* @param string $path
300+
* @return string
301+
*/
302+
public function normalize_path($path) {
303+
$path = preg_replace('~[/]{2,}~', '/', $path);
304+
$parts = explode('/', $path);
305+
$return_parts = array();
306+
307+
foreach($parts as $current_part) {
308+
if($current_part === '..') {
309+
if(!empty($return_parts) && end($return_parts) !== '') {
310+
array_pop($return_parts);
311+
}
312+
} else {
313+
$return_parts[] = $current_part;
314+
}
315+
}
316+
317+
return implode('/', $return_parts);
318+
}
319+
320+
293321
/** IDN converter wrapper.
294322
* all converter classes should be placed in ISPC_CLASS_PATH.'/idn/'
295323
*/
@@ -370,42 +398,42 @@ public function idn_decode($domain) {
370398

371399
public function is_allowed_user($username, $restrict_names = false) {
372400
global $app;
373-
401+
374402
$name_blacklist = array('root','ispconfig','vmail','getmail');
375403
if(in_array($username,$name_blacklist)) return false;
376-
404+
377405
if(preg_match('/^[a-zA-Z0-9\.\-_]{1,32}$/', $username) == false) return false;
378-
406+
379407
if($restrict_names == true && preg_match('/^web\d+$/', $username) == false) return false;
380-
408+
381409
return true;
382410
}
383-
411+
384412
public function is_allowed_group($groupname, $restrict_names = false) {
385413
global $app;
386-
414+
387415
$name_blacklist = array('root','ispconfig','vmail','getmail');
388416
if(in_array($groupname,$name_blacklist)) return false;
389-
417+
390418
if(preg_match('/^[a-zA-Z0-9\.\-_]{1,32}$/', $groupname) == false) return false;
391-
419+
392420
if($restrict_names == true && preg_match('/^client\d+$/', $groupname) == false) return false;
393-
421+
394422
return true;
395423
}
396-
424+
397425
public function getimagesizefromstring($string){
398426
if (!function_exists('getimagesizefromstring')) {
399427
$uri = 'data://application/octet-stream;base64,' . base64_encode($string);
400428
return getimagesize($uri);
401429
} else {
402430
return getimagesizefromstring($string);
403-
}
431+
}
404432
}
405-
433+
406434
public function password($minLength = 10, $special = false){
407435
global $app;
408-
436+
409437
$iteration = 0;
410438
$password = "";
411439
$maxLength = $minLength + 5;
@@ -430,21 +458,21 @@ public function password($minLength = 10, $special = false){
430458
public function getRandomInt($min, $max){
431459
return floor((mt_rand() / mt_getrandmax()) * ($max - $min + 1)) + $min;
432460
}
433-
461+
434462
public function generate_customer_no(){
435463
global $app;
436464
// generate customer no.
437465
$customer_no = mt_rand(100000, 999999);
438466
while($app->db->queryOneRecord("SELECT client_id FROM client WHERE customer_no = ?", $customer_no)) {
439467
$customer_no = mt_rand(100000, 999999);
440468
}
441-
469+
442470
return $customer_no;
443471
}
444-
472+
445473
public function generate_ssh_key($client_id, $username = ''){
446474
global $app;
447-
475+
448476
// generate the SSH key pair for the client
449477
$id_rsa_file = '/tmp/'.uniqid('',true);
450478
$id_rsa_pub_file = $id_rsa_file.'.pub';
@@ -458,7 +486,7 @@ public function generate_ssh_key($client_id, $username = ''){
458486
$app->log("Failed to create SSH keypair for ".$username, LOGLEVEL_WARN);
459487
}
460488
}
461-
489+
462490
public function htmlentities($value) {
463491
global $conf;
464492

@@ -474,10 +502,10 @@ public function htmlentities($value) {
474502
} else {
475503
$out = htmlentities($value, ENT_QUOTES, $conf["html_content_encoding"]);
476504
}
477-
505+
478506
return $out;
479507
}
480-
508+
481509
// Function to check paths before we use it as include. Use with absolute paths only.
482510
public function check_include_path($path) {
483511
if(strpos($path,'//') !== false) die('Include path seems to be an URL: '.$this->htmlentities($path));
@@ -488,18 +516,18 @@ public function check_include_path($path) {
488516
if(substr($path,0,strlen(ISPC_ROOT_PATH)) != ISPC_ROOT_PATH) die('Path '.$this->htmlentities($path).' is outside of ISPConfig installation directory.');
489517
return $path;
490518
}
491-
519+
492520
// Function to check language strings
493521
public function check_language($language) {
494522
global $app;
495523
if(preg_match('/^[a-z]{2}$/',$language)) {
496524
return $language;
497525
} else {
498526
$app->log('Wrong language string: '.$this->htmlentities($language),1);
499-
return 'en';
527+
return 'en';
500528
}
501529
}
502-
530+
503531
}
504532

505533
?>

‎interface/lib/classes/tform_base.inc.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ function applyValueLimit($formtype, $limit, $values, $current_value = '') {
399399
$tmp_key = $limit_parts[2];
400400
$allowed = $allowed = explode(',',$tmp_conf[$tmp_key]);
401401
}
402-
402+
403403
if($formtype == 'CHECKBOX') {
404404
if(strstr($limit,'force_')) {
405405
// Force the checkbox field to be ticked and enabled
@@ -958,6 +958,9 @@ function filterField($field_name, $field_value, $filters, $filter_event) {
958958
case 'STRIPNL':
959959
$returnval = str_replace(array("\n","\r"),'', $returnval);
960960
break;
961+
case 'NORMALIZEPATH':
962+
$returnval = $app->functions->normalize_path($returnval);
963+
break;
961964
default:
962965
$this->errorMessage .= "Unknown Filter: ".$filter['type'];
963966
break;

‎interface/web/sites/form/shell_user.tform.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,12 @@
232232
'dir' => array (
233233
'datatype' => 'VARCHAR',
234234
'formtype' => 'TEXT',
235+
'filters' => array(
236+
0 => array (
237+
'event' => 'SAVE',
238+
'type' => 'NORMALIZEPATH'
239+
)
240+
),
235241
'validators' => array ( 0 => array ( 'type' => 'NOTEMPTY',
236242
'errmsg'=> 'directory_error_empty'),
237243
1 => array ( 'type' => 'REGEX',

‎server/lib/classes/functions.inc.php‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,34 @@ public function intval($string, $force_numeric = false) {
356356
}
357357
}
358358

359+
360+
/**
361+
* Normalize a path and strip duplicate slashes from it
362+
*
363+
* This will also remove all /../ from the path, reducing the preceding path elements
364+
*
365+
* @param string $path
366+
* @return string
367+
*/
368+
public function normalize_path($path) {
369+
$path = preg_replace('~[/]{2,}~', '/', $path);
370+
$parts = explode('/', $path);
371+
$return_parts = array();
372+
373+
foreach($parts as $current_part) {
374+
if($current_part === '..') {
375+
if(!empty($return_parts) && end($return_parts) !== '') {
376+
array_pop($return_parts);
377+
}
378+
} else {
379+
$return_parts[] = $current_part;
380+
}
381+
}
382+
383+
return implode('/', $return_parts);
384+
}
385+
386+
359387
/** IDN converter wrapper.
360388
* all converter classes should be placed in ISPC_CLASS_PATH.'/idn/'
361389
*/
@@ -435,10 +463,10 @@ public function idn_decode($domain) {
435463
}
436464
return implode("\n", $domains);
437465
}
438-
466+
439467
public function generate_ssh_key($client_id, $username = ''){
440468
global $app;
441-
469+
442470
// generate the SSH key pair for the client
443471
$id_rsa_file = '/tmp/'.uniqid('',true);
444472
$id_rsa_pub_file = $id_rsa_file.'.pub';

‎server/lib/classes/system.inc.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2300,6 +2300,36 @@ public function is_allowed_group($groupname, $check_id = true, $restrict_names =
23002300
return true;
23012301
}
23022302

2303+
public function is_allowed_path($path) {
2304+
global $app;
2305+
2306+
$path = $app->functions->normalize_path($path);
2307+
if(file_exists($path)) {
2308+
$path = realpath($path);
2309+
}
2310+
2311+
$blacklisted_paths_regex = array(
2312+
'@^/$@',
2313+
'@^/proc(/.*)?$@',
2314+
'@^/sys(/.*)?$@',
2315+
'@^/etc(/.*)?$@',
2316+
'@^/dev(/.*)?$@',
2317+
'@^/tmp(/.*)?$@',
2318+
'@^/run(/.*)?$@',
2319+
'@^/boot(/.*)?$@',
2320+
'@^/root(/.*)?$@',
2321+
'@^/var(/?|/backups?(/.*)?)?$@',
2322+
);
2323+
2324+
foreach($blacklisted_paths_regex as $regex) {
2325+
if(preg_match($regex, $path)) {
2326+
return false;
2327+
}
2328+
}
2329+
2330+
return true;
2331+
}
2332+
23032333
public function last_exec_out() {
23042334
return $this->_last_exec_out;
23052335
}
@@ -2350,6 +2380,8 @@ public function system_safe($cmd) {
23502380
}
23512381

23522382
public function create_jailkit_user($username, $home_dir, $user_home_dir, $shell = '/bin/bash', $p_user = null, $p_user_home_dir = null) {
2383+
global $app;
2384+
23532385
// Disallow operating on root directory
23542386
if(realpath($home_dir) == '/') {
23552387
$app->log("create_jailkit_user: invalid home_dir: $home_dir", LOGLEVEL_WARN);
@@ -2379,6 +2411,8 @@ public function create_jailkit_user($username, $home_dir, $user_home_dir, $shell
23792411
}
23802412

23812413
public function create_jailkit_chroot($home_dir, $app_sections = array(), $options = array()) {
2414+
global $app;
2415+
23822416
// Disallow operating on root directory
23832417
if(realpath($home_dir) == '/') {
23842418
$app->log("create_jailkit_chroot: invalid home_dir: $home_dir", LOGLEVEL_WARN);
@@ -2450,6 +2484,8 @@ public function create_jailkit_chroot($home_dir, $app_sections = array(), $optio
24502484
}
24512485

24522486
public function create_jailkit_programs($home_dir, $programs = array(), $options = array()) {
2487+
global $app;
2488+
24532489
// Disallow operating on root directory
24542490
if(realpath($home_dir) == '/') {
24552491
$app->log("create_jailkit_programs: invalid home_dir: $home_dir", LOGLEVEL_WARN);

‎server/plugins-available/shelluser_base_plugin.inc.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,14 @@ function insert($event_name, $data) {
9696
return false;
9797
}
9898

99+
if(is_file($data['new']['dir']) || is_link($data['new']['dir'])) {
100+
$app->log('Shell user dir must not be existing file or symlink.', LOGLEVEL_WARN);
101+
return false;
102+
} elseif(!$app->system->is_allowed_path($data['new']['dir'])) {
103+
$app->log('Shell user dir is not an allowed path: ' . $data['new']['dir'], LOGLEVEL_WARN);
104+
return false;
105+
}
106+
99107
if($data['new']['active'] != 'y' || $data['new']['chroot'] == "jailkit") $data['new']['shell'] = '/bin/false';
100108

101109
if($app->system->is_user($data['new']['puser'])) {
@@ -207,6 +215,14 @@ function update($event_name, $data) {
207215
return false;
208216
}
209217

218+
if(is_file($data['new']['dir']) || is_link($data['new']['dir'])) {
219+
$app->log('Shell user dir must not be existing file or symlink.', LOGLEVEL_WARN);
220+
return false;
221+
} elseif(!$app->system->is_allowed_path($data['new']['dir'])) {
222+
$app->log('Shell user dir is not an allowed path: ' . $data['new']['dir'], LOGLEVEL_WARN);
223+
return false;
224+
}
225+
210226
if($data['new']['active'] != 'y') $data['new']['shell'] = '/bin/false';
211227

212228
if($app->system->is_user($data['new']['puser'])) {
@@ -304,6 +320,14 @@ function delete($event_name, $data) {
304320
return false;
305321
}
306322

323+
if(is_file($data['old']['dir']) || is_link($data['old']['dir'])) {
324+
$app->log('Shell user dir must not be existing file or symlink.', LOGLEVEL_WARN);
325+
return false;
326+
} elseif(!$app->system->is_allowed_path($data['old']['dir'])) {
327+
$app->log('Shell user dir is not an allowed path: ' . $data['old']['dir'], LOGLEVEL_WARN);
328+
return false;
329+
}
330+
307331
if($app->system->is_user($data['old']['username'])) {
308332
// Get the UID of the user
309333
$userid = intval($app->system->getuid($data['old']['username']));

0 commit comments

Comments
 (0)