Permalink
Browse files

CVE-2013-5954: Fixed CSRF vulnerabilities

Multiple scripts were performing actions upon GET calls without any
protection from CSRF attacks. Under certain circumstances, that could
allow attackers to delete entities on a Revive Adserver instance.

Forms using POST had been fixed already.
  • Loading branch information...
1 parent 2b05e99 commit 79cb2db05c9849e225885e8a622978da014a98a7 @andrewatfornax andrewatfornax committed with mbeccati May 2, 2014
View
@@ -110,9 +110,29 @@ function init($templateName)
$this->assign("pluginBaseDir", MAX_PATH.'/www/admin/plugins/');
$this->assign("pluginTemplateDir", '/templates/');
+ /**
+ * CVE-2013-5954
+ *
+ * Register the helper method to allow the the required session token to
+ * be placed into GET method calls for CRUD operations in templates. See
+ * OA_Permission::checkSessionToken() method for details.
+ */
+ $this->register_function('rv_add_session_token', array('OA_Admin_Template', '_add_session_token'));
}
+ /**
+ * CVE-2013-5954
+ *
+ * Helper method to allow the the required session token to be placed
+ * into GET method calls for CRUD operations in templates. See
+ * OA_Permission::checkSessionToken() method for details.
+ */
+ static public function _add_session_token()
+ {
+ return 'token=' . urlencode(phpAds_SessionGetToken());
+ }
+
/**
* A method to set a cache id for the current page
*
View
@@ -81,6 +81,23 @@ class OA_Permission
const OPERATION_VIEW_CHILDREN = 128;
const OPERATION_ALL = 255;//1+2+4+8+16+ 32+64+128
+ /**
+ * CVE-2013-5954
+ *
+ * Helper method which checks if the correct session token is present
+ * when CRUD actions (generally deletes) are performed using a GET instead
+ * of a POST (for historical reasons). Allows the CSRF vulnerabilities
+ * reported in CVE-2013-5954 to be closed off without the required (and
+ * eventually needed) refactoring of the enture UI to a proper MVC
+ * framework.
+ */
+ public static function checkSessionToken()
+ {
+ $token = isset($_GET['token']) ? $_GET['token'] : false;
+ OA_Permission::enforceTrue(
+ phpAds_SessionValidateToken($token)
+ );
+ }
/**
* Helper method which checks whether $condition is true, if it is not true
View
@@ -1300,7 +1300,7 @@ function MAX_displayChannels($channels, $aParams) {
echo "<td height='25' colspan='3'>";
echo "<img src='" . OX::assetPath() . "/images/icon-acl.gif' border='0' align='absmiddle' alt='{$GLOBALS['strIncludedBanners']}'>&nbsp;<a href='channel-acl.php?{$entityString}channelid={$channel['channel_id']}'>{$GLOBALS['strEditChannelLimitations']}</a>&nbsp;&nbsp;&nbsp;&nbsp;";
- echo "<img src='" . OX::assetPath() . "/images/icon-recycle.gif' border='0' align='absmiddle' alt='{$GLOBALS['strDelete']}'>&nbsp;<a href='channel-delete.php?{$entityString}channelid={$channel['channel_id']}&returnurl=".(empty($aParams['affiliateid']) ? 'channel-index.php' : 'affiliate-channels.php')."'".phpAds_DelConfirm($GLOBALS['strConfirmDeleteChannel']).">{$GLOBALS['strDelete']}</a>&nbsp;&nbsp;&nbsp;&nbsp;";
+ echo "<img src='" . OX::assetPath() . "/images/icon-recycle.gif' border='0' align='absmiddle' alt='{$GLOBALS['strDelete']}'>&nbsp;<a href='channel-delete.php?token=" . urlencode(phpAds_SessionGetToken()) . "&{$entityString}channelid={$channel['channel_id']}&returnurl=".(empty($aParams['affiliateid']) ? 'channel-index.php' : 'affiliate-channels.php')."'".phpAds_DelConfirm($GLOBALS['strConfirmDeleteChannel']).">{$GLOBALS['strDelete']}</a>&nbsp;&nbsp;&nbsp;&nbsp;";
echo "</td></tr>";
$i++;
@@ -1704,7 +1704,7 @@ function addTrackerPageTools($advertiserId, $trackerId, $aOtherAdvertisers)
//delete
$deleteConfirm = phpAds_DelConfirm($GLOBALS['strConfirmDeleteTracker']);
- addPageLinkTool($GLOBALS["strDelete"], MAX::constructUrl(MAX_URL_ADMIN, "tracker-delete.php?clientid=".$advertiserId."&trackerid=".$trackerId."&returnurl=advertiser-trackers.php"), "iconDelete", null, $deleteConfirm);
+ addPageLinkTool($GLOBALS["strDelete"], MAX::constructUrl(MAX_URL_ADMIN, "tracker-delete.php?token=" . urlencode(phpAds_SessionGetToken()) . "&clientid=".$advertiserId."&trackerid=".$trackerId."&returnurl=advertiser-trackers.php"), "iconDelete", null, $deleteConfirm);
addPageShortcut($GLOBALS['strBackToTrackers'], MAX::constructUrl(MAX_URL_ADMIN, "advertiser-trackers.php?clientid=$advertiserId"), "iconBack");
}
}
@@ -1738,7 +1738,7 @@ function addCampaignPageTools($clientid, $campaignid, $aOtherAdvertisers, $aEnti
}
$deleteConfirm = phpAds_DelConfirm($GLOBALS['strConfirmDeleteCampaign']);
- addPageLinkTool($GLOBALS["strDelete"], MAX::constructUrl(MAX_URL_ADMIN, "campaign-delete.php?clientid=$clientid&campaignid=$campaignid&returnurl=advertiser-campaigns.php"), "iconDelete", null, $deleteConfirm);
+ addPageLinkTool($GLOBALS["strDelete"], MAX::constructUrl(MAX_URL_ADMIN, "campaign-delete.php?token=" . urlencode(phpAds_SessionGetToken()) . "&clientid=$clientid&campaignid=$campaignid&returnurl=advertiser-campaigns.php"), "iconDelete", null, $deleteConfirm);
}
//shortcuts
@@ -1813,7 +1813,7 @@ function addBannerPageTools($advertiserId, $campaignId, $bannerId, $aOtherCampai
//delete
if (!OA_Permission::isAccount(OA_ACCOUNT_ADVERTISER)) {
$deleteConfirm = phpAds_DelConfirm($GLOBALS['strConfirmDeleteBanner']);
- addPageLinkTool($GLOBALS["strDelete"], MAX::constructUrl(MAX_URL_ADMIN, "banner-delete.php?clientid=$advertiserId&campaignid=$campaignId&bannerid=$bannerId&returnurl=campaign-banners.php"), "iconDelete", null, $deleteConfirm);
+ addPageLinkTool($GLOBALS["strDelete"], MAX::constructUrl(MAX_URL_ADMIN, "banner-delete.php?token=" . urlencode(phpAds_SessionGetToken()) . "&clientid=$advertiserId&campaignid=$campaignId&bannerid=$bannerId&returnurl=campaign-banners.php"), "iconDelete", null, $deleteConfirm);
}
/* Shortcuts */
@@ -1870,7 +1870,7 @@ function addZonePageTools($affiliateid, $zoneid, $aOtherPublishers, $aEntities)
|| OA_Permission::isAccount(OA_ACCOUNT_MANAGER)
|| OA_Permission::hasPermission(OA_PERM_ZONE_DELETE)) {
$deleteConfirm = phpAds_DelConfirm($GLOBALS['strConfirmDeleteZone']);
- addPageLinkTool($GLOBALS["strDelete"], MAX::constructUrl(MAX_URL_ADMIN, "zone-delete.php?affiliateid=$affiliateid&zoneid=$zoneid&returnurl=affiliate-zones.php"), "iconDelete", null, $deleteConfirm);
+ addPageLinkTool($GLOBALS["strDelete"], MAX::constructUrl(MAX_URL_ADMIN, "zone-delete.php?token=" . urlencode(phpAds_SessionGetToken()) . "&affiliateid=$affiliateid&zoneid=$zoneid&returnurl=affiliate-zones.php"), "iconDelete", null, $deleteConfirm);
}
//shortcut
@@ -1893,7 +1893,7 @@ function addChannelPageTools($agencyid, $websiteId, $channelid, $channelType)
//delete
$deleteConfirm = phpAds_DelConfirm($GLOBALS['strConfirmDeleteChannel']);
- addPageLinkTool($GLOBALS["strDelete"], MAX::constructUrl(MAX_URL_ADMIN, "channel-delete.php?&agencyid=$agencyid&affiliateid=$websiteId&channelid=$channelid&returnurl=$deleteReturlUrl"), "iconDelete", null, $deleteConfirm);
+ addPageLinkTool($GLOBALS["strDelete"], MAX::constructUrl(MAX_URL_ADMIN, "channel-delete.php?token=" . urlencode(phpAds_SessionGetToken()) . "&agencyid=$agencyid&affiliateid=$websiteId&channelid=$channelid&returnurl=$deleteReturlUrl"), "iconDelete", null, $deleteConfirm);
}
@@ -7,7 +7,7 @@
| Copyright: See the COPYRIGHT.txt file. |
| License: GPLv2 or later, see the LICENSE.txt file. |
+---------------------------------------------------------------------------+
-
+
-->*}
<!-- Top -->
@@ -89,7 +89,7 @@
</td>
<td height='25'>
- <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("advertiser-delete.php?clientid={$row_clients.clientid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteClient}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
+ <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("advertiser-delete.php?{rv_add_session_token}&clientid={$row_clients.clientid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteClient}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
</td>
<td height='25'>
@@ -117,7 +117,7 @@
</td>
<td height='25'>
- <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("campaign-delete.php?clientid={$row_clients.clientid|escape}&campaignid={$row_c_expand.campaignid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteCampaign}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
+ <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("campaign-delete.php?{rv_add_session_token}&clientid={$row_clients.clientid|escape}&campaignid={$row_c_expand.campaignid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteCampaign}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
</td>
<td height='25'>
@@ -145,7 +145,7 @@
</td>
<td height='25'>
- <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("banner-delete.php?clientid={$row_clients.clientid|escape}&campaignid={$row_b_expand.campaignid|escape}&bannerid={$row_b_expand.bannerid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteBanner}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
+ <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("banner-delete.php?clientid={$row_clients.clientid|escape}&campaignid={$row_b_expand.campaignid|escape}&{rv_add_session_token}&bannerid={$row_b_expand.bannerid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteBanner}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
</td>
<td height='25'>
@@ -182,7 +182,7 @@
</td>
<td height='25'>
- <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("campaign-delete.php?clientid={$row_campaigns.clientid|escape}&campaignid={$row_campaigns.campaignid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteCampaign}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
+ <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("campaign-delete.php?{rv_add_session_token}&clientid={$row_campaigns.clientid|escape}&campaignid={$row_campaigns.campaignid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteCampaign}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
</td>
<td height='25'>
@@ -210,7 +210,7 @@
</td>
<td height='25'>
- <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("banner-delete.php?clientid={$row_campaigns.clientid|escape}&campaignid={$row_b_expand.campaignid|escape}&bannerid={$row_b_expand.bannerid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteBanner}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
+ <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("banner-delete.php?clientid={$row_campaigns.clientid|escape}&campaignid={$row_b_expand.campaignid|escape}&{rv_add_session_token}&bannerid={$row_b_expand.bannerid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteBanner}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
</td>
<td height='25'>
@@ -245,7 +245,7 @@
</td>
<td height='25'>
- <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("banner-delete.php?clientid={$row_banners.clientid|escape}&campaignid={$row_banners.campaignid|escape}&bannerid={$row_banners.bannerid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteBanner}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
+ <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("banner-delete.php?clientid={$row_banners.clientid|escape}&campaignid={$row_banners.campaignid|escape}&{rv_add_session_token}&bannerid={$row_banners.bannerid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteBanner}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
</td>
<td height='25'>
@@ -279,7 +279,7 @@
</td>
<td height='25'>
- <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("affiliate-delete.php?affiliateid={$row_affiliates.affiliateid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteAffiliate}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
+ <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("affiliate-delete.php?{rv_add_session_token}&affiliateid={$row_affiliates.affiliateid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteAffiliate}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
</td>
<td height='25'>
@@ -307,7 +307,7 @@
</td>
<td height='25'>
- <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("zone-delete.php?affiliateid={$row_z_expand.affiliateid|escape}&zoneid={$row_z_expand.zoneid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteZone}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
+ <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("zone-delete.php?{rv_add_session_token}&affiliateid={$row_z_expand.affiliateid|escape}&zoneid={$row_z_expand.zoneid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteZone}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
</td>
<td height='25'>
@@ -342,7 +342,7 @@
</td>
<td height='25'>
- <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("zone-delete.php?affiliateid={$row_zones.affiliateid|escape}&zoneid={$row_zones.zoneid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteZone}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
+ <img src='{$assetPath|escape}/images/icon-recycle.gif' border='0' align='absmiddle' alt='{t str=Delete}'>&nbsp;<a href='JavaScript:GoOpener("zone-delete.php?{rv_add_session_token}&affiliateid={$row_zones.affiliateid|escape}&zoneid={$row_zones.zoneid|escape:js}", true)'{phpAds_DelConfirm str=ConfirmDeleteZone}>{t str=Delete}</a>&nbsp;&nbsp;&nbsp;&nbsp;
</td>
<td height='25'>
@@ -19,41 +19,41 @@
</li>
<li class='inactive activeIfSelected'>
<a id='deleteSelection' href='#' class='inlineIcon iconDelete'>{t str=Delete}</a>
-
+
{literal}
<script type='text/javascript'>
<!--
-
+
$('#deleteSelection').click(function(event) {
event.preventDefault();
-
+
if (!$(this).parents('li').hasClass('inactive')) {
var ids = [];
$(this).parents('.tableWrapper').find('.toggleSelection input:checked').each(function() {
ids.push(this.value);
});
-
+
if (!tablePreferences.warningBeforeDelete || confirm("{/literal}{t str=ConfirmDeleteClients}{literal}")) {
- window.location = 'advertiser-delete.php?clientid=' + ids.join(',');
+ window.location = 'advertiser-delete.php?{/literal}{rv_add_session_token}{literal}&clientid=' + ids.join(',');
}
}
});
-
+
//-->
</script>
{/literal}
</li>
</ul>
-
+
<ul class='tableFilters alignRight'>
<li>
<div class='label'>
Show
</div>
-
+
<div class='dropDown'>
<span><span>{if $hideinactive}Active advertisers{else}All advertisers{/if}</span></span>
-
+
<div class='panel'>
<div>
<ul>
@@ -62,7 +62,7 @@
</ul>
</div>
</div>
-
+
<div class='mask'></div>
</div>
</li>
@@ -91,7 +91,7 @@
{ox_column_title item=name order=up default=1 str=Name url=advertiser-index.php}
</th>
<th class='last alignRight'>&nbsp;
-
+
</th>
</tr>
</thead>
@@ -105,28 +105,28 @@
<td colspan='3' class="hasPanel">
<div class='tableMessage'>
<div class='panel'>
-
+
{if $hideinactive}
{$aCount.advertisers_hidden} {t str=InactiveAdvertisersHidden}
{else}
{t str=$listEmptyStr|default:"NoClients"}
{/if}
-
+
<div class='corner top-left'></div>
<div class='corner top-right'></div>
<div class='corner bottom-left'></div>
<div class='corner bottom-right'></div>
</div>
</div>
-
+
&nbsp;
</td>
</tr>
<tr class='odd'>
<td colspan='3'>&nbsp;</td>
</tr>
</tbody>
-
+
{else}
<tbody>
{cycle name=bgcolor values="even,odd" assign=bgColor reset=1}
@@ -144,7 +144,7 @@
{else}
<a href='advertiser-edit.php?clientid={$clientId}' class='inlineIcon iconAdvertiser'>{$client.clientname|escape:html}</a>
{ox_entity_id type="Advertiser" id=$clientId}
- {/if}
+ {/if}
</td>
<td class='alignRight horizontalActions'>
<ul class='rowActions'>
Oops, something went wrong.

0 comments on commit 79cb2db

Please sign in to comment.