Skip to content

Commit

Permalink
Fix h1 report 107879
Browse files Browse the repository at this point in the history 
Reflected XSS
-------------

Johan Caluwe has reported via HackerOne that www/admin/stats.php was
vulnerable to reflected XSS attacks via multiple parameters that were not
properly sanitised or escaped when displayed, such as "setPerPage", "pageId",
"bannerid", "pereiod_start", "period_end" and possibly others.

A CVE-ID has been requested, but not assigned yet.

CWE: CWE-79
CVSSv2: 5.6 (AV:N/AC:H/Au:S/C:C/I:P/A:N)
  • Loading branch information
@mbeccati
mbeccati committed Mar 1, 2016
1 parent 5a48f3e commit ecbe822
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 52 deletions.
9 changes: 5 additions & 4 deletions lib/max/other/html.php
View file
Expand Up @@ -192,15 +192,16 @@ function MAX_displayNoStatsMessage()
function _getHtmlHeaderColumn($title, $name, $pageName, $entityIds, $listorder, $orderdirection, $showColumn = true)
{
$str = '';
$entity = _getEntityString($entityIds);
$entity = htmlspecialchars(_getEntityString($entityIds), ENT_QUOTES);
$pageName = htmlspecialchars($pageName, ENT_QUOTES);
if ($listorder == $name) {
if (($orderdirection == '') || ($orderdirection == 'down')) {
$str = "<a href='$pageName?{$entity}orderdirection=up'><img src='" . OX::assetPath() . "/images/caret-ds.gif' border='0' alt='' title=''></a>";
} else {
$str = "<a href='$pageName?{$entity}orderdirection=down'><img src='" . OX::assetPath() . "/images/caret-u.gif' border='0' alt='' title=''></a>";
}
}
return $showColumn ? "<b><a href='$pageName?{$entity}listorder=$name'>$title</a>$str</b>" : '';
return $showColumn ? "<b><a href='$pageName?{$entity}listorder=".urlencode($name)."'>$title</a>$str</b>" : '';
}

function _getEntityString($entityIds)
Expand All @@ -209,9 +210,9 @@ function _getEntityString($entityIds)
if (!empty($entityIds)) {
$entityArr = array();
foreach ($entityIds as $entityId => $entityValue) {
$entityArr[] = "$entityId=$entityValue";
$entityArr[] = "$entityId=".urlencode($entityValue);
}
$entity = implode('&',$entityArr) . '&';
$entity = implode('&', $entityArr) . '&';
}

return $entity;
Expand Down
2 changes: 1 addition & 1 deletion lib/templates/admin/layout/links-container.html
View file
Expand Up @@ -20,7 +20,7 @@
<li>
{if $aLinks[linkLoop].type == 'link'}
{if $aLinks[linkLoop].icon}<img src='{$assetPath}/images/{$aLinks[linkLoop].icon}' align='absmiddle' alt=''>{/if}
<a href='{$aLinks[linkLoop].url}' {if $aLinks[linkLoop].iconClass}class='inlineIcon {$aLinks[linkLoop].iconClass}'{/if} {if $aLinks[linkLoop].accesskey}accesskey='{$aLinks[linkLoop].accesskey}'{/if} {$aLinks[linkLoop].extraAttr}>{$aLinks[linkLoop].title}</a>
<a href='{$aLinks[linkLoop].url|escape}' {if $aLinks[linkLoop].iconClass}class='inlineIcon {$aLinks[linkLoop].iconClass}'{/if} {if $aLinks[linkLoop].accesskey}accesskey='{$aLinks[linkLoop].accesskey}'{/if} {$aLinks[linkLoop].extraAttr}>{$aLinks[linkLoop].title}</a>
{elseif $aLinks[linkLoop].type == 'form'}
{if $aLinks[linkLoop].icon}<img src='{$assetPath}/images/{$aLinks[linkLoop].icon}' align='absmiddle' alt=''>{/if}
<label {if $aLinks[linkLoop].iconClass}class='inlineIcon {$aLinks[linkLoop].iconClass}'{/if}>{$aLinks[linkLoop].title}</label>
Expand Down
97 changes: 50 additions & 47 deletions www/admin/stats-conversions.php
View file
Expand Up @@ -58,46 +58,49 @@
$expand = MAX_getValue('expand', '');
$collapse = MAX_getValue('collapse');

$clientId = MAX_getValue('clientid');
$campaignId = MAX_getValue('campaignid');
$bannerId = MAX_getValue('bannerid');

$affiliateId = MAX_getValue('affiliateid');
$zoneId = MAX_getValue('zoneid');

if (OA_Permission::isAccount(OA_ACCOUNT_ADVERTISER)) {
$clientId = $clientid = OA_Permission::getEntityId();
} elseif (OA_Permission::isAccount(OA_ACCOUNT_TRAFFICKER)) {
$affiliateId = $affiliateid = OA_Permission::getEntityId();
if ($clientid) {
OA_Permission::enforceAccessToObject('clients', $clientid);
}
if ($campaignid) {
OA_Permission::enforceAccessToObject('campaigns', $campaignid);
}
if ($bannerid) {
OA_Permission::enforceAccessToObject('banners', $bannerid);
}
if ($affiliateid) {
OA_Permission::enforceAccessToObject('affiliates', $clientid);
}
if ($zoneid) {
OA_Permission::enforceAccessToObject('zones', $zoneid);
}

// Build $addUrl variable which will be added to any required link on this page, eg: expand, collapse, editStatuses
$entityIds = array(
'entity' => 'conversions',
'clientid' => $clientid,
'campaignid' => $campaignId,
'bannerid' => $bannerId,
'affiliateid' => $affiliateId,
'zoneid' => $zoneId,
'campaignid' => $campaignid,
'bannerid' => $bannerid,
'affiliateid' => $affiliateid,
'zoneid' => $zoneid,
'setPerPage' => $setPerPage,
'pageID' => $pageID
);
$addUrl = "entity=conversions&clientid=$clientId&campaignid=$campaignId&bannerid=$bannerId&affiliateid=$affiliateId&zoneid=$zoneId&setPerPage=$setPerPage&pageID=$pageID";
$addUrl = "entity=conversions&clientid=$clientid&campaignid=$campaignid&bannerid=$bannerid&affiliateid=$affiliateid&zoneid=$zoneid&setPerPage=$setPerPage&pageID=$pageID";

if (!empty($day)) {
$entityIds += array(
'day' => $day,
'hour' => $hour,
'howLong' => $howLong
);
$addUrl .= "&day={$day}&hour={$hour}&howLong={$howLong}";
$addUrl .= "&day=".urlencode($day)."&hour=".urlencode($hour)."&howLong=".urlencode($howLong);
} else {
$entityIds += array(
'period_preset' => $period_preset,
'period_start' => $period_start,
'period_end' => $period_end,
);
$addUrl .= "&period_preset={$period_preset}&period_start={$period_start}&period_end={$period_end}";
$addUrl .= "&period_preset=".urlencode($period_preset)."&period_start=".urlencode($period_start)."&period_end=".urlencode($period_end);
}
// Adjust which nodes are opened closed...
MAX_adjustNodes($aNodes, $expand, $collapse);
Expand Down Expand Up @@ -178,11 +181,11 @@

$hiddenValues = array(
'entity' => 'conversions',
'clientid' => $clientId,
'campaignid' => $campaignId,
'bannerid' => $bannerId,
'affiliateid' => $affiliateId,
'zoneid' => $zoneId,
'clientid' => $clientid,
'campaignid' => $campaignid,
'bannerid' => $bannerid,
'affiliateid' => $affiliateid,
'zoneid' => $zoneid,
);
if(!empty($period_preset)) {
MAX_displayDateSelectionForm($period_preset, $period_start, $period_end, $pageName, $tabindex, $hiddenValues);
Expand All @@ -199,15 +202,15 @@
$aParams = array();
$aParams['agency_id'] = OA_Permission::getAgencyId();

$aParams['clientid'] = $clientId;
$aParams['campaignid'] = $campaignId;
$aParams['bannerid'] = $bannerId;
$aParams['clientid'] = $clientid;
$aParams['campaignid'] = $campaignid;
$aParams['bannerid'] = $bannerid;
$aZonesIds = null; // Admin_DA class expects null if no zones to be used
if (empty($zoneId) && !empty($affiliateId)) {
$aZonesIds = Admin_DA::fromCache('getZonesIdsByAffiliateId', $affiliateId);
if (empty($zoneid) && !empty($affiliateid)) {
$aZonesIds = Admin_DA::fromCache('getZonesIdsByAffiliateId', $affiliateid);
}
if(!empty($zoneId)) {
$aZonesIds = array($zoneId);
if(!empty($zoneid)) {
$aZonesIds = array($zoneid);
}
$aParams['zonesIds'] = $aZonesIds;

Expand Down Expand Up @@ -251,23 +254,23 @@

if($editStatuses) {
echo "<form id='connections-modify' action='connections-modify.php' name='connectionsmodify' id='connectionsmodify' method='POST'>"."\n";
echo "<input type='hidden' name='clientid' value='$clientId'>"."\n";
echo "<input type='hidden' name='campaignid' value='$campaignId'>"."\n";
echo "<input type='hidden' name='bannerid' value='$bannerId'>"."\n";
echo "<input type='hidden' name='affiliateid' value='$affiliateId'>"."\n";
echo "<input type='hidden' name='zoneid' value='$zoneId'>"."\n";
echo "<input type='hidden' name='day' value='$day'>"."\n";
echo "<input type='hidden' name='hour' value='$hour'>"."\n";
echo "<input type='hidden' name='howLong' value='$howLong'>"."\n";
echo "<input type='hidden' name='period_preset' value='$period_preset'>"."\n";
echo "<input type='hidden' name='clientid' value='$clientid'>"."\n";
echo "<input type='hidden' name='campaignid' value='$campaignid'>"."\n";
echo "<input type='hidden' name='bannerid' value='$bannerid'>"."\n";
echo "<input type='hidden' name='affiliateid' value='$affiliateid'>"."\n";
echo "<input type='hidden' name='zoneid' value='$zoneid'>"."\n";
echo "<input type='hidden' name='day' value='".htmlspecialchars($day, ENT_QUOTES)."'>"."\n";
echo "<input type='hidden' name='hour' value='".htmlspecialchars($hour, ENT_QUOTES)."'>"."\n";
echo "<input type='hidden' name='howLong' value='".htmlspecialchars($howLong, ENT_QUOTES)."'>"."\n";
echo "<input type='hidden' name='period_preset' value='".htmlspecialchars($period_preset, ENT_QUOTES)."'>"."\n";
if ($period_preset == 'specific') {
echo "<input type='hidden' name='period_start' value='$period_start'>"."\n";
echo "<input type='hidden' name='period_end' value='$period_end'>"."\n";
echo "<input type='hidden' name='period_start' value='".htmlspecialchars($period_start, ENT_QUOTES)."'>"."\n";
echo "<input type='hidden' name='period_end' value='".htmlspecialchars($period_end, ENT_QUOTES)."'>"."\n";
}
echo "<input type='hidden' name='returnurl' value='stats.php'>"."\n";
echo "<input type='hidden' name='entity' value='conversions'>"."\n";
echo "<input type='hidden' name='setPerPage' value='$setPerPage'>"."\n";
echo "<input type='hidden' name='pageID' value='$pageID'>"."\n";
echo "<input type='hidden' name='setPerPage' value='".htmlspecialchars($setPerPage, ENT_QUOTES)."'>"."\n";
echo "<input type='hidden' name='pageID' value='".htmlspecialchars($pageID, ENT_QUOTES)."'>"."\n";
}

echo "
Expand Down Expand Up @@ -331,9 +334,9 @@
<tr height='25'$bgcolor>
<td>";
if ($conversionExpanded) {
echo "&nbsp;<a href='$pageName?collapse=a$conversionId&$addUrl'><img src='" . OX::assetPath() . "/images/triangle-d.gif' align='absmiddle' border='0'></a>&nbsp;";
echo "&nbsp;<a href='".htmlspecialchars("$pageName?collapse=a$conversionId&$addUrl", ENT_QUOTES)."'><img src='" . OX::assetPath() . "/images/triangle-d.gif' align='absmiddle' border='0'></a>&nbsp;";
} else {
echo "&nbsp;<a href='$pageName?expand=a$conversionId&$addUrl'><img src='" . OX::assetPath() . "/images/$phpAds_TextDirection/triangle-l.gif' align='absmiddle' border='0'></a>&nbsp;";
echo "&nbsp;<a href='".htmlspecialchars("$pageName?expand=a$conversionId&$addUrl", ENT_QUOTES)."'><img src='" . OX::assetPath() . "/images/$phpAds_TextDirection/triangle-l.gif' align='absmiddle' border='0'></a>&nbsp;";
}

$aConversionStatuses = array(
Expand Down Expand Up @@ -444,7 +447,7 @@
<td colspan='4' align='$phpAds_TextAlignLeft' nowrap>";
echo "
</td>
<td colspan='2' align='$phpAds_TextAlignRight' nowrap><img src='" . OX::assetPath() . "/images/triangle-d.gif' align='absmiddle' border='0'>&nbsp;<a href='$pageName?$addUrl&amp;expand=all' accesskey='$keyExpandAll'>$strExpandAll</a>&nbsp;&nbsp;|&nbsp;&nbsp;<img src='" . OX::assetPath() . "/images/$phpAds_TextDirection/triangle-l.gif' align='absmiddle' border='0'>&nbsp;<a href='$pageName?$addUrl&amp;expand=none' accesskey='$keyCollapseAll'>$strCollapseAll</a>&nbsp;&nbsp;</td>
<td colspan='2' align='$phpAds_TextAlignRight' nowrap><img src='" . OX::assetPath() . "/images/triangle-d.gif' align='absmiddle' border='0'>&nbsp;<a href='".htmlspecialchars("$pageName?$addUrl&expand=all", ENT_QUOTES)."' accesskey='$keyExpandAll'>$strExpandAll</a>&nbsp;&nbsp;|&nbsp;&nbsp;<img src='" . OX::assetPath() . "/images/$phpAds_TextDirection/triangle-l.gif' align='absmiddle' border='0'>&nbsp;<a href='$pageName?$addUrl&amp;expand=none' accesskey='$keyCollapseAll'>$strCollapseAll</a>&nbsp;&nbsp;</td>
</tr>";
echo "<tr>
Expand All @@ -469,7 +472,7 @@
echo "</form>"."\n";
}

echo "<form id='setPager' method='get' action='stats.php?".htmlentities($_SERVER['QUERY_STRING'])."'>";
echo "<form id='setPager' method='get' action='stats.php?".htmlspecialchars($_SERVER['QUERY_STRING'], ENT_QUOTES)."'>";

$getValues = preg_split('/&/D', $_SERVER['QUERY_STRING']);
foreach ($getValues as $record) {
Expand Down

0 comments on commit ecbe822

Please sign in to comment.