Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

This closes bug #351 'Security Risk - Download Reports page can inadv…

…ertently download unapproved reports'
  • Loading branch information...
commit 34cf544e05cbf80a4e35cc60e9e0a8c786b4c64e 1 parent 6442b91
John Etherton jetherton authored
Showing with 50 additions and 8 deletions.
  1. +50 −8 application/controllers/admin/reports.php
58 application/controllers/admin/reports.php
View
@@ -914,33 +914,75 @@ public function download()
// Test to see if things passed the rule checks
if ($post->validate())
{
- // Add Filters
- $filter = " ( 1 !=1";
-
+ //set filter
+ $filter = '( ';
+
// Report Type Filter
+ $show_active = false;
+ $show_inactive = false;
+ $show_verified = false;
+ $show_not_verified = false;
foreach($post->data_point as $item)
{
if ($item == 1)
{
- $filter .= " OR incident_active = 1 ";
+ $show_active = true;
}
if ($item == 2)
{
- $filter .= " OR incident_verified = 1 ";
+ $show_verified = true;
}
if ($item == 3)
{
- $filter .= " OR incident_active = 0 ";
+ $show_inactive = true;
}
if ($item == 4)
{
- $filter .= " OR incident_verified = 0 ";
+ $show_not_verified = true;
}
}
- $filter .= ") ";
+ //handle active or not active
+ if($show_active && !$show_inactive)
+ {
+ $filter .= ' incident_active = 1 ';
+ }
+ elseif(!$show_active && $show_inactive)
+ {
+ $filter .= ' incident_active = 0 ';
+ }
+ elseif($show_active && $show_inactive)
+ {
+ $filter .= ' (incident_active = 1 OR incident_active = 1) ';
Robbie Mackay Owner

I assume this was actually meant to be:
$filter .= ' (incident_active = 1 OR incident_active = 0) ';

John Etherton Collaborator
Robbie Mackay Owner

No worries - looking at the next block, I guess you just got the checks backwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+ elseif(!$show_active && !$show_inactive)
+ {
+ $filter .= ' (incident_active = 0 AND incident_active = 1) ';
Robbie Mackay Owner

Shouldn't this be just

$filter .= ' (incident_active = 1) ';

so default to showing only active incidents if neither is selected?

John Etherton Collaborator
Robbie Mackay Owner

Yes. My point was it should be returning ONLY active.. not active or inactive in this case.

John Etherton Collaborator

Thanks Robbie, I've made a fix to this fix here: 9656e74

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+
+ $filter .= ' AND ';
+
+ //handle verified
+ if($show_verified && !$show_not_verified)
+ {
+ $filter .= ' incident_verified = 1 ';
+ }
+ elseif(!$show_verified && $show_not_verified)
+ {
+ $filter .= ' incident_verified = 0 ';
+ }
+ elseif($show_verified && $show_not_verified)
+ {
+ $filter .= ' (incident_verified = 0 OR incident_verified = 1) ';
+ }
+ elseif(!$show_verified && !$show_not_verified)
+ {
+ $filter .= ' (incident_verified = 0 AND incident_verified = 1) ';
+ }
+
+ $filter .= ') ';
// Report Date Filter
if ( ! empty($post->from_date) AND !empty($post->to_date))
Robbie Mackay

I assume this was actually meant to be:
$filter .= ' (incident_active = 1 OR incident_active = 0) ';

Robbie Mackay

Shouldn't this be just

$filter .= ' (incident_active = 1) ';

so default to showing only active incidents if neither is selected?

John Etherton
John Etherton
Robbie Mackay

No worries - looking at the next block, I guess you just got the checks backwards.

Robbie Mackay

Yes. My point was it should be returning ONLY active.. not active or inactive in this case.

John Etherton

Thanks Robbie, I've made a fix to this fix here: 9656e74

Please sign in to comment.
Something went wrong with that request. Please try again.