Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fixed SQL injection issue in _Load_Users
That was probably the only place in the software that had SQL injection since
we took the sort value and put it directly into the query.  The sort value
is now verified as a valid column.

This wasn't really an issue however since the JSON entrypoint was only accessible
via an admin account anyway.
  • Loading branch information
ummmmm committed Aug 21, 2022
1 parent 1612e24 commit dd77a35
Showing 1 changed file with 31 additions and 6 deletions.
37 changes: 31 additions & 6 deletions html/includes/runtime/admin/JSON/LoadUsers.php
Expand Up @@ -6,19 +6,19 @@ public function execute()
{
$sort = Functions::Post( 'sort' );
$direction = Functions::Post( 'direction' );

if ( !$this->_Load_Users( $sort, $direction, $users ) )
{
return $this->setDBError();
return false;
}

foreach( $users as &$loaded_user )
{
$loaded_user[ 'last_on' ] = Functions::FormatDate( $loaded_user[ 'last_on' ] );
$loaded_user[ 'current_place' ] = Functions::Place( $loaded_user[ 'current_place' ] );
$loaded_user[ 'current_place' ] = Functions::Place( $loaded_user[ 'current_place' ] );
}

return $this->setData( $users );
return $this->setData( $users );
}

// Helper functions
Expand All @@ -28,6 +28,25 @@ private function _Load_Users( $sort, $direction, &$users )
$db_weeks = new Weeks( $this->_db );
$current = $db_weeks->Current();
$direction = ( $direction === 'asc' ) ? 'ASC' : 'DESC';

switch ( $sort )
{
case 'name' :
case 'current_place' :
case 'last_on' :
case 'paid' :
case 'failed_logins' :
case 'active_sessions' :
case 'remaining' :
{
break;
}
default :
{
return $this->setError( array( '#Error#', 'Invalid sort' ) );
}
}

$sql = "SELECT
u.*,
CONCAT( u.fname, ' ', u.lname ) AS name,
Expand All @@ -45,6 +64,12 @@ private function _Load_Users( $sort, $direction, &$users )
u.id
ORDER BY
{$sort} {$direction}";
return $this->_db->select( $sql, $users, $current );

if ( !$this->_db->select( $sql, $users, $current ) )
{
return $this->setDBError();
}

return true;
}
}

0 comments on commit dd77a35

Please sign in to comment.