Skip to content
Permalink
Browse files Browse the repository at this point in the history
security: Audit Log Injection
This mitigates a vulnerability discovered by the AppSec Research Team at
Checkmarx where it's possible to perform injection via Audit Log plugin.
This is due to passing the `order` URL param directly to the select query.
This refactors the `getOrder()` method to only return predefined sort orders
to prevent using user-input.
  • Loading branch information
JediKev committed Apr 21, 2022
1 parent 8b33116 commit 0b59afb
Showing 1 changed file with 10 additions and 6 deletions.
16 changes: 10 additions & 6 deletions audit/class.audit.php
Expand Up @@ -463,12 +463,17 @@ static function getQwhere($objectId, $hide_views=false, $type='') {
}

static function getOrder($order) {
if($_REQUEST['order'] && $orderWays[strtoupper($_REQUEST['order'])]) {
$order=$orderWays[strtoupper($_REQUEST['order'])];
}
$order=$order?$order:'DESC';
$or = null;
$orderWays=array('DESC'=>'DESC','ASC'=>'ASC');

if ($order && $orderWays[strtoupper($order)])
$or = $orderWays[strtoupper($order)];
elseif($_REQUEST['order'] && $orderWays[strtoupper($_REQUEST['order'])])
$or = $orderWays[strtoupper($_REQUEST['order'])];

$or = $or ? $or : 'DESC';

return $order;
return $or;
}

static function getQuery($qs, $objectId, $pageNav, $export, $type='') {
Expand All @@ -478,7 +483,6 @@ static function getQuery($qs, $objectId, $pageNav, $export, $type='') {

$sortOptions=array('id'=>'audit.id', 'object_id'=>'audit.object_id', 'state'=>'audit.state','type'=>'audit.object_type','ip'=>'audit.ip'
,'timestamp'=>'audit.timestamp');
$orderWays=array('DESC'=>'DESC','ASC'=>'ASC');
$sort=($_REQUEST['sort'] && $sortOptions[strtolower($_REQUEST['sort'])])?strtolower($_REQUEST['sort']):'timestamp';
//Sorting options...
if($sort && $sortOptions[$sort]) {
Expand Down

0 comments on commit 0b59afb

Please sign in to comment.