Skip to content

Commit 8feac17

Browse files
author
Andrew Bauer
authored
Merge pull request #1764 from ZoneMinder/vulerability-fixes
sql injection and session fixation vulerability fixes
2 parents 9e9b1a3 + 5804cd2 commit 8feac17

File tree

7 files changed

+124
-70
lines changed

7 files changed

+124
-70
lines changed

‎web/ajax/log.php‎

Lines changed: 91 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
<?php
22

3+
# Moved up here because it is used in several spots.
4+
# These are the valid columns that you can filter on.
5+
$filterFields = array( 'Component', 'ServerId', 'Pid', 'Level', 'File', 'Line' );
6+
37
switch ( $_REQUEST['task'] )
48
{
59
case 'create' :
@@ -31,68 +35,85 @@
3135
if ( !canView( 'System' ) )
3236
ajaxError( 'Insufficient permissions to view log entries' );
3337

34-
$servers = Server::find_all();
35-
$servers_by_Id = array();
36-
# There is probably a better way to do this.
37-
foreach ( $servers as $server ) {
38-
$servers_by_Id[$server->Id()] = $server;
39-
}
38+
$servers = Server::find_all();
39+
$servers_by_Id = array();
40+
# There is probably a better way to do this.
41+
foreach ( $servers as $server ) {
42+
$servers_by_Id[$server->Id()] = $server;
43+
}
4044

4145
$minTime = isset($_POST['minTime'])?$_POST['minTime']:NULL;
4246
$maxTime = isset($_POST['maxTime'])?$_POST['maxTime']:NULL;
43-
$limit = isset($_POST['limit'])?$_POST['limit']:100;
44-
$filter = isset($_POST['filter'])?$_POST['filter']:array();
45-
$sortField = isset($_POST['sortField'])?$_POST['sortField']:'TimeKey';
47+
$limit = 100;
48+
if ( isset($_POST['limit']) ) {
49+
if ( ( !is_integer( $_POST['limit'] ) and !ctype_digit($_POST['limit']) ) ) {
50+
Error("Invalid value for limit " . $_POST['limit'] );
51+
} else {
52+
$limit = $_POST['limit'];
53+
}
54+
}
55+
$sortField = 'TimeKey';
56+
if ( isset($_POST['sortField']) ) {
57+
if ( ! in_array( $_POST['sortField'], $filterFields ) and ( $_POST['sortField'] != 'TimeKey' ) ) {
58+
Error("Invalid sort field " . $_POST['sortField'] );
59+
} else {
60+
$sortField = $_POST['sortField'];
61+
}
62+
}
4663
$sortOrder = (isset($_POST['sortOrder']) and $_POST['sortOrder']) == 'asc' ? 'asc':'desc';
64+
$filter = isset($_POST['filter'])?$_POST['filter']:array();
4765

48-
$filterFields = array( 'Component', 'ServerId', 'Pid', 'Level', 'File', 'Line' );
49-
50-
$total = dbFetchOne( "SELECT count(*) AS Total FROM Logs", 'Total' );
66+
$total = dbFetchOne( 'SELECT count(*) AS Total FROM Logs', 'Total' );
5167
$sql = 'SELECT * FROM Logs';
5268
$where = array();
53-
$values = array();
69+
$values = array();
5470
if ( $minTime ) {
55-
$where[] = "TimeKey > ?";
56-
$values[] = $minTime;
71+
$where[] = "TimeKey > ?";
72+
$values[] = $minTime;
5773
} elseif ( $maxTime ) {
58-
$where[] = "TimeKey < ?";
59-
$values[] = $maxTime;
60-
}
74+
$where[] = "TimeKey < ?";
75+
$values[] = $maxTime;
76+
}
77+
6178
foreach ( $filter as $field=>$value ) {
62-
if ( $field == 'Level' ){
63-
$where[] = $field." <= ?";
64-
$values[] = $value;
65-
} else {
66-
$where[] = $field." = ?";
67-
$values[] = $value;
68-
}
69-
}
79+
if ( ! in_array( $field, $filterFields ) ) {
80+
Error("$field is not in valid filter fields");
81+
continue;
82+
}
83+
if ( $field == 'Level' ){
84+
$where[] = $field." <= ?";
85+
$values[] = $value;
86+
} else {
87+
$where[] = $field." = ?";
88+
$values[] = $value;
89+
}
90+
}
7091
if ( count($where) )
71-
$sql.= ' WHERE '.join( ' AND ', $where );
92+
$sql.= ' WHERE '.join( ' AND ', $where );
7293
$sql .= " order by ".$sortField." ".$sortOrder." limit ".$limit;
7394
$logs = array();
7495
foreach ( dbFetchAll( $sql, NULL, $values ) as $log ) {
7596
$log['DateTime'] = preg_replace( '/^\d+/', strftime( "%Y-%m-%d %H:%M:%S", intval($log['TimeKey']) ), $log['TimeKey'] );
76-
$log['Server'] = ( $log['ServerId'] and isset($servers_by_Id[$log['ServerId']]) ) ? $servers_by_Id[$log['ServerId']]->Name() : '';
97+
$log['Server'] = ( $log['ServerId'] and isset($servers_by_Id[$log['ServerId']]) ) ? $servers_by_Id[$log['ServerId']]->Name() : '';
7798
$logs[] = $log;
7899
}
79100
$options = array();
80101
$where = array();
81-
$values = array();
102+
$values = array();
82103
foreach( $filter as $field=>$value ) {
83104
if ( $field == 'Level' ) {
84105
$where[$field] = $field." <= ?";
85-
$values[$field] = $value;
106+
$values[$field] = $value;
86107
} else {
87108
$where[$field] = $field." = ?";
88-
$values[$field] = $value;
89-
}
90-
}
109+
$values[$field] = $value;
110+
}
111+
}
91112
foreach( $filterFields as $field )
92113
{
93114
$sql = "SELECT DISTINCT $field FROM Logs WHERE NOT isnull($field)";
94115
$fieldWhere = array_diff_key( $where, array( $field=>true ) );
95-
$fieldValues = array_diff_key( $values, array( $field=>true ) );
116+
$fieldValues = array_diff_key( $values, array( $field=>true ) );
96117
if ( count($fieldWhere) )
97118
$sql.= " AND ".join( ' AND ', $fieldWhere );
98119
$sql.= " ORDER BY $field ASC";
@@ -108,7 +129,7 @@
108129
{
109130
foreach( dbFetchAll( $sql, $field, array_values($fieldValues) ) as $value )
110131
$options['ServerId'][$value] = ( $value and isset($servers_by_Id[$value]) ) ? $servers_by_Id[$value]->Name() : '';
111-
132+
112133
}
113134
else
114135
{
@@ -147,44 +168,51 @@
147168
}
148169
//$limit = isset($_POST['limit'])?$_POST['limit']:1000;
149170
$filter = isset($_POST['filter'])?$_POST['filter']:array();
150-
$sortField = isset($_POST['sortField'])?$_POST['sortField']:'TimeKey';
151-
$sortOrder = isset($_POST['sortOrder'])?$_POST['sortOrder']:'asc';
171+
$sortField = 'TimeKey';
172+
if ( isset($_POST['sortField']) ) {
173+
if ( ! in_array( $_POST['sortField'], $filterFields ) and ( $_POST['sortField'] != 'TimeKey' ) ) {
174+
Error("Invalid sort field " . $_POST['sortField'] );
175+
} else {
176+
$sortField = $_POST['sortField'];
177+
}
178+
}
179+
$sortOrder = (isset($_POST['sortOrder']) and $_POST['sortOrder']) == 'asc' ? 'asc':'desc';
152180

153-
$servers = Server::find_all();
154-
$servers_by_Id = array();
155-
# There is probably a better way to do this.
156-
foreach ( $servers as $server ) {
157-
$servers_by_Id[$server->Id()] = $server;
158-
}
181+
$servers = Server::find_all();
182+
$servers_by_Id = array();
183+
# There is probably a better way to do this.
184+
foreach ( $servers as $server ) {
185+
$servers_by_Id[$server->Id()] = $server;
186+
}
159187

160188
$sql = "select * from Logs";
161189
$where = array();
162-
$values = array();
190+
$values = array();
163191
if ( $minTime )
164192
{
165193
preg_match( '/(.+)(\.\d+)/', $minTime, $matches );
166194
$minTime = strtotime($matches[1]).$matches[2];
167195
$where[] = "TimeKey >= ?";
168-
$values[] = $minTime;
196+
$values[] = $minTime;
169197
}
170198
if ( $maxTime )
171199
{
172200
preg_match( '/(.+)(\.\d+)/', $maxTime, $matches );
173201
$maxTime = strtotime($matches[1]).$matches[2];
174202
$where[] = "TimeKey <= ?";
175-
$values[] = $maxTime;
203+
$values[] = $maxTime;
176204
}
177205
foreach ( $filter as $field=>$value ) {
178206
if ( $value != '' ) {
179207
if ( $field == 'Level' ) {
180208
$where[] = $field." <= ?";
181-
$values[] = $value;
209+
$values[] = $value;
182210
} else {
183211
$where[] = $field." = ?'";
184-
$values[] = $value;
185-
}
186-
}
187-
}
212+
$values[] = $value;
213+
}
214+
}
215+
}
188216
if ( count($where) )
189217
$sql.= " where ".join( " and ", $where );
190218
$sql .= " order by ".$sortField." ".$sortOrder;
@@ -216,7 +244,7 @@
216244
foreach ( dbFetchAll( $sql, NULL, $values ) as $log )
217245
{
218246
$log['DateTime'] = preg_replace( '/^\d+/', strftime( "%Y-%m-%d %H:%M:%S", intval($log['TimeKey']) ), $log['TimeKey'] );
219-
$log['Server'] = ( $log['ServerId'] and isset($servers_by_Id[$log['ServerId']]) ) ? $servers_by_Id[$log['ServerId']]->Name() : '';
247+
$log['Server'] = ( $log['ServerId'] and isset($servers_by_Id[$log['ServerId']]) ) ? $servers_by_Id[$log['ServerId']]->Name() : '';
220248
$logs[] = $log;
221249
}
222250
switch( $format )
@@ -234,20 +262,20 @@
234262
}
235263
case 'tsv' :
236264
{
237-
# This line doesn't need fprintf, it could use fwrite
265+
# This line doesn't need fprintf, it could use fwrite
238266
fprintf( $exportFP, join( "\t",
239-
translate('DateTime'),
240-
translate('Component'),
241-
translate('Server'),
242-
translate('Pid'),
243-
translate('Level'),
244-
translate('Message'),
245-
translate('File'),
246-
translate('Line')
247-
)."\n" );
267+
translate('DateTime'),
268+
translate('Component'),
269+
translate('Server'),
270+
translate('Pid'),
271+
translate('Level'),
272+
translate('Message'),
273+
translate('File'),
274+
translate('Line')
275+
)."\n" );
248276
foreach ( $logs as $log )
249277
{
250-
fprintf( $exportFP, "%s\t%s\t%s\t%d\t%s\t%s\t%s\t%s\n", $log['DateTime'], $log['Component'], $log['Server'], $log['Pid'], $log['Code'], $log['Message'], $log['File'], $log['Line'] );
278+
fprintf( $exportFP, "%s\t%s\t%s\t%d\t%s\t%s\t%s\t%s\n", $log['DateTime'], $log['Component'], $log['Server'], $log['Pid'], $log['Code'], $log['Message'], $log['File'], $log['Line'] );
251279
}
252280
break;
253281
}

‎web/includes/Frame.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ class Frame {
66
public function __construct( $IdOrRow ) {
77
$row = NULL;
88
if ( $IdOrRow ) {
9-
if ( is_integer( $IdOrRow ) or is_numeric( $IdOrRow ) ) {
9+
if ( is_integer( $IdOrRow ) or ctype_digit($IdOrRow) ) {
1010
$row = dbFetchOne( 'SELECT * FROM Frames WHERE Id=?', NULL, array( $IdOrRow ) );
1111
if ( ! $row ) {
1212
Error("Unable to load Frame record for Id=" . $IdOrRow );
@@ -84,7 +84,15 @@ function($v){ return $v.'=?'; },
8484
$values = array_values( $parameters );
8585
}
8686
if ( $limit ) {
87-
$sql .= ' LIMIT ' . $limit;
87+
if ( is_integer( $limit ) or ctype_digit( $limit ) ) {
88+
$sql .= ' LIMIT ' . $limit;
89+
} else {
90+
$backTrace = debug_backtrace();
91+
$file = $backTrace[1]['file'];
92+
$line = $backTrace[1]['line'];
93+
Error("Invalid value for limit($limit) passed to Frame::find from $file:$line");
94+
return;
95+
}
8896
}
8997
$results = dbFetchAll( $sql, NULL, $values );
9098
if ( $results ) {

‎web/includes/Server.php‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ class Server {
55
public function __construct( $IdOrRow = NULL ) {
66
$row = NULL;
77
if ( $IdOrRow ) {
8-
if ( is_integer( $IdOrRow ) or is_numeric( $IdOrRow ) ) {
8+
if ( is_integer( $IdOrRow ) or ctype_digit( $IdOrRow ) ) {
99
$row = dbFetchOne( 'SELECT * FROM Servers WHERE Id=?', NULL, array( $IdOrRow ) );
1010
if ( ! $row ) {
1111
Error("Unable to load Server record for Id=" . $IdOrRow );
@@ -63,9 +63,15 @@ function($v){ return $v.'=?'; },
6363
) );
6464
$values = array_values( $parameters );
6565
}
66-
if ( $limit ) {
67-
$sql .= ' LIMIT ' . $limit;
68-
}
66+
if ( is_integer( $limit ) or ctype_digit( $limit ) ) {
67+
$sql .= ' LIMIT ' . $limit;
68+
} else {
69+
$backTrace = debug_backtrace();
70+
$file = $backTrace[1]['file'];
71+
$line = $backTrace[1]['line'];
72+
Error("Invalid value for limit($limit) passed to Server::find from $file:$line");
73+
return;
74+
}
6975
$results = dbFetchAll( $sql, NULL, $values );
7076
if ( $results ) {
7177
return array_map( function($id){ return new Server($id); }, $results );

‎web/includes/database.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ function dbConnect()
4444

4545
try {
4646
$dbConn = new PDO( ZM_DB_TYPE . $socket . ';dbname='.ZM_DB_NAME, ZM_DB_USER, ZM_DB_PASS );
47+
$dbConn->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
4748
$dbConn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
4849
} catch(PDOException $ex ) {
4950
echo "Unable to connect to ZM db." . $ex->getMessage();

‎web/includes/functions.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ function userLogin( $username, $password="", $passwordHashed=false ) {
5656
if ( ZM_AUTH_TYPE == "builtin" ) {
5757
$_SESSION['passwordHash'] = $user['Password'];
5858
}
59+
session_regenerate_id();
5960
} else {
6061
Warning( "Login denied for user \"$username\"" );
6162
$_SESSION['loginFailed'] = true;

‎web/includes/logger.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ function Error( $string )
528528
function Fatal( $string )
529529
{
530530
Logger::fetch()->logPrint( Logger::FATAL, $string );
531-
die( $string );
531+
die( htmlentities($string) );
532532
}
533533

534534
function Panic( $string )

‎web/index.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,16 @@
112112
Fatal( "Invalid skin '$skin'" );
113113
$skinBase[] = $skin;
114114

115+
$currentCookieParams = session_get_cookie_params();
116+
Debug('Setting cookie parameters to lifetime('.$currentCookieParams['lifetime'].') path('.$currentCookieParams['path'].') domain ('.$currentCookieParams['domain'].') secure('.$currentCookieParams['secure'].') httpOnly(1)');
117+
session_set_cookie_params(
118+
$currentCookieParams["lifetime"],
119+
$currentCookieParams["path"],
120+
$currentCookieParams["domain"],
121+
$currentCookieParams["secure"],
122+
true
123+
);
124+
115125
ini_set( "session.name", "ZMSESSID" );
116126

117127
session_start();

0 commit comments

Comments
 (0)