Skip to content

Commit 139bbca

Browse files
author
Till Brehm
committed
Various security improvments in API and server code.
1 parent 2285eec commit 139bbca

File tree

9 files changed

+222
-49
lines changed

9 files changed

+222
-49
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ Development branch: [![pipeline status](https://git.ispconfig.org/ispconfig/ispc
2626
[^1]: not actively tested
2727

2828
## Supported operating systems
29-
- Debian 9, 11, and testing
29+
- Debian 9 - 11, and testing
3030
- Ubuntu 16.04 - 20.04
3131
- CentOS 7 and 8
3232

‎install/sql/incremental/upd_dev_collection.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE `remote_session` ADD `remote_ip` VARCHAR(39) NOT NULL DEFAULT '' AFTER `tstamp`;

‎install/sql/ispconfig3.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1317,6 +1317,7 @@ CREATE TABLE `remote_session` (
13171317
`remote_functions` text,
13181318
`client_login` tinyint(1) unsigned NOT NULL default '0',
13191319
`tstamp` int(10) unsigned NOT NULL DEFAULT '0',
1320+
`remote_ip` varchar(39) NOT NULL DEFAULT '',
13201321
PRIMARY KEY (`remote_session`)
13211322
) DEFAULT CHARSET=utf8 ;
13221323

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

Lines changed: 105 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,20 @@ public function login($username, $password, $client_login = false)
8585
//* Delete old remoting sessions
8686
$sql = "DELETE FROM remote_session WHERE tstamp < UNIX_TIMESTAMP()";
8787
$app->db->query($sql);
88+
89+
//* Check for max. login attempts
90+
$ip_md5 = md5($_SERVER['REMOTE_ADDR']);
91+
$sql = "SELECT * FROM `attempts_login` WHERE `ip`= ? AND `login_time` > (NOW() - INTERVAL 5 MINUTE) LIMIT 1";
92+
$alreadyfailed = $app->db->queryOneRecord($sql, $ip_md5);
93+
94+
if($alreadyfailed['times'] > 10) {
95+
throw new SoapFault('login_failure_limit', 'The login failure limit has been reached.');
96+
return false;
97+
}
8898

8999
if($client_login == true) {
90100
$sql = "SELECT * FROM sys_user WHERE USERNAME = ?";
91-
$user = $app->db->queryOneRecord($sql, $username);
101+
$user = $app->db->queryOneRecord($sql, (string)$username);
92102
if($user) {
93103
$saved_password = stripslashes($user['passwort']);
94104

@@ -104,6 +114,16 @@ public function login($username, $password, $client_login = false)
104114
}
105115
}
106116
} else {
117+
if(!$alreadyfailed['times'] )
118+
{
119+
//* user login the first time wrong
120+
$sql = "INSERT INTO `attempts_login` (`ip`, `times`, `login_time`) VALUES (?, 1, NOW())";
121+
$app->db->query($sql, $ip_md5);
122+
} elseif($alreadyfailed['times'] >= 1) {
123+
//* update times wrong
124+
$sql = "UPDATE `attempts_login` SET `times`=`times`+1, `login_time`=NOW() WHERE `ip` = ? AND `login_time` < NOW() ORDER BY `login_time` DESC LIMIT 1";
125+
$app->db->query($sql, $ip_md5);
126+
}
107127
throw new SoapFault('client_login_failed', 'The login failed. Username or password wrong.');
108128
}
109129
if($user['active'] != 1) {
@@ -119,17 +139,23 @@ public function login($username, $password, $client_login = false)
119139

120140
//* Create a remote user session
121141
//srand ((double)microtime()*1000000);
122-
$remote_session = md5(mt_rand().uniqid('ispco'));
142+
$remote_session = substr(str_shuffle('abcdefghijklmnopqrstuvwxyz'),0,1).sha1(mt_rand().uniqid('ispco',true));
123143
$remote_userid = $user['userid'];
124144
$remote_functions = '';
125145
$tstamp = time() + $this->session_timeout;
126-
$sql = 'INSERT INTO remote_session (remote_session,remote_userid,remote_functions,client_login,tstamp'
127-
.') VALUES (?, ?, ?, 1, ?)';
128-
$app->db->query($sql, $remote_session,$remote_userid,$remote_functions,$tstamp);
146+
$ip = $_SERVER['REMOTE_ADDR'];
147+
$sql = 'INSERT INTO remote_session (remote_session,remote_userid,remote_functions,client_login,tstamp,remote_ip'
148+
.') VALUES (?, ?, ?, 1, ?, ?)';
149+
$app->db->query($sql, $remote_session,$remote_userid,$remote_functions,$tstamp,$ip);
150+
151+
//* Delete login attempts after successful login
152+
$sql = "DELETE FROM `attempts_login` WHERE `ip`=?";
153+
$app->db->query($sql, $ip_md5);
154+
129155
return $remote_session;
130156
} else {
131157
$sql = "SELECT * FROM remote_user WHERE remote_username = ?";
132-
$remote_user = $app->db->queryOneRecord($sql, $username);
158+
$remote_user = $app->db->queryOneRecord($sql, (string)$username);
133159
if($remote_user) {
134160
if(substr($remote_user['remote_password'], 0, 1) === '$') {
135161
if(crypt(stripslashes($password), $remote_user['remote_password']) != $remote_user['remote_password']) {
@@ -138,7 +164,7 @@ public function login($username, $password, $client_login = false)
138164
} elseif(md5($password) == $remote_user['remote_password']) {
139165
// update hash algo
140166
$sql = 'UPDATE `remote_user` SET `remote_password` = ? WHERE `remote_username` = ?';
141-
$app->db->query($sql, $app->auth->crypt_password($password), $username);
167+
$app->db->query($sql, $app->auth->crypt_password($password), (string)$username);
142168
} else {
143169
$remote_user = null;
144170
}
@@ -185,15 +211,32 @@ public function login($username, $password, $client_login = false)
185211
}
186212
//* Create a remote user session
187213
//srand ((double)microtime()*1000000);
188-
$remote_session = md5(mt_rand().uniqid('ispco'));
214+
$remote_session = substr(str_shuffle('abcdefghijklmnopqrstuvwxyz'),0,1).sha1(mt_rand().uniqid('ispco',true));
189215
$remote_userid = $remote_user['remote_userid'];
190216
$remote_functions = $remote_user['remote_functions'];
191217
$tstamp = time() + $this->session_timeout;
192-
$sql = 'INSERT INTO remote_session (remote_session,remote_userid,remote_functions,tstamp'
193-
.') VALUES (?, ?, ?, ?)';
194-
$app->db->query($sql, $remote_session,$remote_userid,$remote_functions,$tstamp);
218+
$sql = 'INSERT INTO remote_session (remote_session,remote_userid,remote_functions,tstamp,remote_ip'
219+
.') VALUES (?, ?, ?, ?, ?)';
220+
$app->db->query($sql, $remote_session,$remote_userid,$remote_functions,$tstamp, $ip);
221+
222+
//* Delete login attempts after successful login
223+
$sql = "DELETE FROM `attempts_login` WHERE `ip`=?";
224+
$app->db->query($sql, $ip_md5);
225+
195226
return $remote_session;
196227
} else {
228+
229+
if(!$alreadyfailed['times'] )
230+
{
231+
//* user login the first time wrong
232+
$sql = "INSERT INTO `attempts_login` (`ip`, `times`, `login_time`) VALUES (?, 1, NOW())";
233+
$app->db->query($sql, $ip_md5);
234+
} elseif($alreadyfailed['times'] >= 1) {
235+
//* update times wrong
236+
$sql = "UPDATE `attempts_login` SET `times`=`times`+1, `login_time`=NOW() WHERE `ip` = ? AND `login_time` < NOW() ORDER BY `login_time` DESC LIMIT 1";
237+
$app->db->query($sql, $ip_md5);
238+
}
239+
197240
throw new SoapFault('login_failed', 'The login failed. Username or password wrong.');
198241
return false;
199242
}
@@ -212,7 +255,7 @@ public function logout($session_id)
212255
}
213256

214257
$sql = "DELETE FROM remote_session WHERE remote_session = ?";
215-
if($app->db->query($sql, $session_id) != false) {
258+
if($app->db->query($sql, (string)$session_id) != false) {
216259
return true;
217260
} else {
218261
return false;
@@ -522,12 +565,61 @@ protected function getSession($session_id)
522565
throw new SoapFault('session_id_empty', 'The SessionID is empty.');
523566
return false;
524567
}
568+
569+
if(!is_string($session_id)) {
570+
throw new SoapFault('session_id_nostring', 'Wrong SessionID datatype.');
571+
return false;
572+
}
573+
574+
$ip_md5 = md5($_SERVER['REMOTE_ADDR']);
575+
$sql = "SELECT * FROM `attempts_login` WHERE `ip`= ? AND `login_time` > (NOW() - INTERVAL 5 MINUTE) LIMIT 1";
576+
$alreadyfailed = $app->db->queryOneRecord($sql, $ip_md5);
577+
578+
if($alreadyfailed['times'] > 10) {
579+
throw new SoapFault('session_failure_limit', 'The Session failure limit has been reached.');
580+
return false;
581+
}
525582

526583
$sql = "SELECT * FROM remote_session WHERE remote_session = ? AND tstamp >= UNIX_TIMESTAMP()";
527-
$session = $app->db->queryOneRecord($sql, $session_id);
584+
$session = $app->db->queryOneRecord($sql, (string)$session_id);
585+
586+
if(!is_array($session)) {
587+
if(!$alreadyfailed['times'] )
588+
{
589+
//* user login the first time wrong
590+
$sql = "INSERT INTO `attempts_login` (`ip`, `times`, `login_time`) VALUES (?, 1, NOW())";
591+
$app->db->query($sql, $ip_md5);
592+
} elseif($alreadyfailed['times'] >= 1) {
593+
//* update times wrong
594+
$sql = "UPDATE `attempts_login` SET `times`=`times`+1, `login_time`=NOW() WHERE `ip` = ? AND `login_time` < NOW() ORDER BY `login_time` DESC LIMIT 1";
595+
$app->db->query($sql, $ip_md5);
596+
}
597+
598+
throw new SoapFault('session_does_not_exist', 'The Session is expired or does not exist.');
599+
return false;
600+
}
601+
602+
$ip = $_SERVER['REMOTE_ADDR'];
603+
if($session['remote_ip'] != $ip) {
604+
throw new SoapFault('session_ip_mismatch', 'Session IP mismatch.');
605+
return false;
606+
}
607+
528608
if($session['remote_userid'] > 0) {
529609
return $session;
530610
} else {
611+
612+
if(!$alreadyfailed['times'] )
613+
{
614+
//* user login the first time wrong
615+
$sql = "INSERT INTO `attempts_login` (`ip`, `times`, `login_time`) VALUES (?, 1, NOW())";
616+
$app->db->query($sql, $ip_md5);
617+
} elseif($alreadyfailed['times'] >= 1) {
618+
//* update times wrong
619+
$sql = "UPDATE `attempts_login` SET `times`=`times`+1, `login_time`=NOW() WHERE `ip` = ? AND `login_time` < NOW() ORDER BY `login_time` DESC LIMIT 1";
620+
$app->db->query($sql, $ip_md5);
621+
}
622+
531623
throw new SoapFault('session_does_not_exist', 'The Session is expired or does not exist.');
532624
return false;
533625
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ function getDataRecord($primary_id, $client_id = 0) {
232232
if(@is_numeric($primary_id)) {
233233
if($primary_id > 0) {
234234
// Return a single record
235-
return parent::getDataRecord($primary_id);
235+
return parent::getDataRecord(intval($primary_id));
236236
} elseif($primary_id == -1) {
237237
// Return a array with all records
238238
$sql = "SELECT * FROM ??";
@@ -255,8 +255,8 @@ function getDataRecord($primary_id, $client_id = 0) {
255255
} else {
256256
$sql_where .= "?? = ? AND ";
257257
}
258-
$params[] = $key;
259-
$params[] = $val;
258+
$params[] = (string)$key;
259+
$params[] = (string)$val;
260260
}
261261
$sql_where = substr($sql_where, 0, -5);
262262
if($sql_where == '') $sql_where = '1';

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ function close () {
7070
function read ($session_id) {
7171

7272
if($this->timeout > 0) {
73-
$rec = $this->db->queryOneRecord("SELECT * FROM sys_session WHERE session_id = ? AND (`permanent` = 'y' OR last_updated >= DATE_SUB(NOW(), INTERVAL ? MINUTE))", $session_id, $this->timeout);
73+
$rec = $this->db->queryOneRecord("SELECT * FROM sys_session WHERE session_id = ? AND (`permanent` = 'y' OR last_updated >= DATE_SUB(NOW(), INTERVAL ? MINUTE))", (string)$session_id, $this->timeout);
7474
} else {
75-
$rec = $this->db->queryOneRecord("SELECT * FROM sys_session WHERE session_id = ?", $session_id);
75+
$rec = $this->db->queryOneRecord("SELECT * FROM sys_session WHERE session_id = ?", (string)$session_id);
7676
}
7777

7878
if (is_array($rec)) {
@@ -91,18 +91,18 @@ function write ($session_id, $session_data) {
9191

9292
// Dont write session_data to DB if session data has not been changed after reading it.
9393
if(isset($this->session_array['session_data']) && $this->session_array['session_data'] != '' && $this->session_array['session_data'] == $session_data) {
94-
$this->db->query("UPDATE sys_session SET last_updated = NOW() WHERE session_id = ?", $session_id);
94+
$this->db->query("UPDATE sys_session SET last_updated = NOW() WHERE session_id = ?", (string)$session_id);
9595
return true;
9696
}
9797

9898

9999
if (@$this->session_array['session_id'] == '') {
100100
$sql = "REPLACE INTO sys_session (session_id,date_created,last_updated,session_data,permanent) VALUES (?,NOW(),NOW(),?,?)";
101-
$this->db->query($sql, $session_id, $session_data, ($this->permanent ? 'y' : 'n'));
101+
$this->db->query($sql, (string)$session_id, $session_data, ($this->permanent ? 'y' : 'n'));
102102

103103
} else {
104104
$sql = "UPDATE sys_session SET last_updated = NOW(), session_data = ?" . ($this->permanent ? ", `permanent` = 'y'" : "") . " WHERE session_id = ?";
105-
$this->db->query($sql, $session_data, $session_id);
105+
$this->db->query($sql, $session_data, (string)$session_id);
106106

107107
}
108108

@@ -112,7 +112,7 @@ function write ($session_id, $session_data) {
112112
function destroy ($session_id) {
113113

114114
$sql = "DELETE FROM sys_session WHERE session_id = ?";
115-
$this->db->query($sql, $session_id);
115+
$this->db->query($sql, (string)$session_id);
116116

117117
return true;
118118
}

‎interface/web/login/index.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@
9696

9797
/* this is the reseller, that shall be re-logged in */
9898
$sql = "SELECT * FROM sys_user WHERE USERNAME = ? and PASSWORT = ?";
99-
$tmp = $app->db->queryOneRecord($sql, $username, $password);
99+
$tmp = $app->db->queryOneRecord($sql, (string)$username, (string)$password);
100100
$client_group_id = $app->functions->intval($tmp['default_group']);
101101
$tmp_client = $app->db->queryOneRecord("SELECT client.client_id FROM sys_group, client WHERE sys_group.client_id = client.client_id and sys_group.groupid = ?", $client_group_id);
102102

@@ -118,7 +118,7 @@
118118

119119
/* this is the user the reseller wants to 'login as' */
120120
$sql = "SELECT * FROM sys_user WHERE USERNAME = ? and PASSWORT = ?";
121-
$tmp = $app->db->queryOneRecord($sql, $username, $password);
121+
$tmp = $app->db->queryOneRecord($sql, (string)$username, (string)$password);
122122
$tmp_client = $app->db->queryOneRecord("SELECT client.client_id, client.parent_client_id FROM sys_group, client WHERE sys_group.client_id = client.client_id and sys_group.groupid = ?", $tmp["default_group"]);
123123

124124
if(!$tmp || $tmp_client["parent_client_id"] != $res_client["client_id"]) {
@@ -146,13 +146,13 @@
146146

147147
if ($loginAs){
148148
$sql = "SELECT * FROM sys_user WHERE USERNAME = ? and PASSWORT = ?";
149-
$user = $app->db->queryOneRecord($sql, $username, $password);
149+
$user = $app->db->queryOneRecord($sql, (string)$username, (string)$password);
150150
} else {
151151

152152
if(stristr($username, '@')) {
153153
//* mailuser login
154154
$sql = "SELECT * FROM mail_user WHERE login = ? or email = ?";
155-
$mailuser = $app->db->queryOneRecord($sql, $username, $app->functions->idn_encode($username));
155+
$mailuser = $app->db->queryOneRecord($sql, (string)$username, $app->functions->idn_encode($username));
156156
$user = false;
157157
if($mailuser) {
158158
$saved_password = stripslashes($mailuser['password']);
@@ -184,7 +184,7 @@
184184
} else {
185185
//* normal cp user login
186186
$sql = "SELECT * FROM sys_user WHERE USERNAME = ?";
187-
$user = $app->db->queryOneRecord($sql, $username);
187+
$user = $app->db->queryOneRecord($sql, (string)$username);
188188
if($user) {
189189
$saved_password = stripslashes($user['passwort']);
190190
if(substr($saved_password, 0, 1) == '$') {
@@ -199,7 +199,7 @@
199199
} else {
200200
// update password with secure algo
201201
$sql = 'UPDATE `sys_user` SET `passwort` = ? WHERE `username` = ?';
202-
$app->db->query($sql, $app->auth->crypt_password($password), $username);
202+
$app->db->query($sql, $app->auth->crypt_password($password), (string)$username);
203203
}
204204
}
205205
} else {

0 commit comments

Comments
 (0)