CVE-2016-9455: Cross-Site Request Forgery (CSRF) (CWE-352)
value: Revive Adserver before 3.2.3 suffers from Cross-Site Request Forgery (CSRF). A number of scripts in Revive Adserver's user interface are vulnerable to CSRF attacks: `www/admin/banner-acl.php`, `www/admin/banner-activate.php`, `www/admin/banner-advanced.php`, `www/admin/banner-modify.php`, `www/admin/banner-swf.php`, `www/admin/banner-zone.php`, `www/admin/tracker-modify.php`.
CWE description: The web application does not, or can not, sufficiently verify whether a well-formed, valid, consistent request was intentionally provided by the user who submitted the request.
CVSS v3 base score: 8.8
Published: 2017-03-28T02:59Z
Repository: https://github.com/revive-adserver/revive-adserver
Language: PHP
Files changed: www/admin/banner-acl.php, www/admin/banner-activate.php, www/admin/banner-advanced.php, www/admin/banner-modify.php, www/admin/banner-swf.php, www/admin/banner-zone.php, www/admin/tracker-modify.php
Commit date: 2016-03-01 16:23:10 +0100
Commit message: Fix h1 report 97123

Cross-Site Request Forgery (CSRF)
---------------------------------

The HackerOne user @decidedlygray has reported a number of scripts in Revive
Adserver's user interface that were vulnerable to CSRF attacks:

- www/admin/banner-acl.php
- www/admin/banner-activate.php
- www/admin/banner-advanced.php
- www/admin/banner-modify.php
- www/admin/banner-swf.php
- www/admin/banner-zone.php
- www/admin/tracker-modify.php

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

CWE: CWE-352
CVSSv2: 5 (AV:N/AC:L/Au:N/C:N/I:N/A:P)

Diff: diff --git a/www/admin/banner-acl.php b/www/admin/banner-acl.php
index 24b4888e63..0e08588642 100644
--- a/www/admin/banner-acl.php
+++ b/www/admin/banner-acl.php
@@ -46,10 +46,12 @@ $tabindex = 1;
 $aEntities = array('clientid' => $clientid, 'campaignid' => $campaignid, 'bannerid' => $bannerid);
 
 if (!empty($action)) {
     $acl = MAX_AclAdjust($acl, $action);
 } elseif (!empty($submit)) {
+    OA_Permission::checkSessionToken();
+
     $acl = (isset($acl)) ? $acl : array();
 
     // Only save when inputs are valid
     if (OX_AclCheckInputsFields($acl, $pageName) === true) {
         $aBannerPrev = MAX_cacheGetAd($bannerid, false);
diff --git a/www/admin/banner-activate.php b/www/admin/banner-activate.php
index 25b46965b5..917079799e 100644
--- a/www/admin/banner-activate.php
+++ b/www/admin/banner-activate.php
@@ -34,10 +34,12 @@ if ($value == OA_ENTITY_STATUS_RUNNING) {
 OA_Permission::enforceAccount(OA_ACCOUNT_MANAGER, OA_ACCOUNT_ADVERTISER);
 OA_Permission::enforceAccessToObject('clients',   $clientid);
 OA_Permission::enforceAccessToObject('campaigns', $campaignid);
 OA_Permission::enforceAccessToObject('banners',   $bannerid, true);
 
+OA_Permission::checkSessionToken();
+
 if (OA_Permission::isAccount(OA_ACCOUNT_ADVERTISER)) {
     if ($value == OA_ENTITY_STATUS_RUNNING) {
         OA_Permission::enforceAllowed(OA_PERM_BANNER_ACTIVATE);
     } else {
         OA_Permission::enforceAllowed(OA_PERM_BANNER_DEACTIVATE);
diff --git a/www/admin/banner-advanced.php b/www/admin/banner-advanced.php
index 15c1e32e24..6845be78e9 100644
--- a/www/admin/banner-advanced.php
+++ b/www/admin/banner-advanced.php
@@ -37,27 +37,30 @@ phpAds_SessionDataStore();
 /*-------------------------------------------------------*/
 /* Process submitted form                                */
 /*-------------------------------------------------------*/
 
 if (isset($submitbutton)) {
-    if (isset($bannerid) && $bannerid != '') {
-        // Update banner
-        $doBanners = OA_Dal::factoryDO('banners');
-        $doBanners->get($bannerid);
-        $doBanners->prepend = $prepend;
-        $doBanners->append  = $append;
-        $doBanners->update();
-
-        // Queue confirmation message
-        $translation = new OX_Translation();
-        $translated_message = $translation->translate($GLOBALS['strBannerAdvancedHasBeenUpdated'], array(
-            MAX::constructURL(MAX_URL_ADMIN, 'banner-edit.php?clientid=' .  $clientid . '&campaignid=' . $campaignid . '&bannerid=' . $bannerid),
-            htmlspecialchars($doBanners->description)
-        ));
-        OA_Admin_UI::queueMessage($translated_message, 'local', 'confirm', 0);
-    }
+    OA_Permission::checkSessionToken();
+
+    // Update banner
+    $doBanners = OA_Dal::factoryDO('banners');
+    $doBanners->get($bannerid);
+    $doBanners->prepend = $prepend;
+    $doBanners->append  = $append;
+    $doBanners->update();
+
+    // Queue confirmation message
+    $translation = new OX_Translation();
+    $translated_message = $translation->translate($GLOBALS['strBannerAdvancedHasBeenUpdated'], array(
+        MAX::constructURL(MAX_URL_ADMIN, 'banner-edit.php?clientid=' .  $clientid . '&campaignid=' . $campaignid . '&bannerid=' . $bannerid),
+        htmlspecialchars($doBanners->description)
+    ));
+
+    OA_Admin_UI::queueMessage($translated_message, 'local', 'confirm', 0);
+
     header ("Location: banner-advanced.php?clientid=".$clientid."&campaignid=".$campaignid."&bannerid=".$bannerid);
+    exit;
 }
 
 /*-------------------------------------------------------*/
 /* HTML framework                                        */
 /*-------------------------------------------------------*/
diff --git a/www/admin/banner-modify.php b/www/admin/banner-modify.php
index f71b237a24..421d79282c 100644
--- a/www/admin/banner-modify.php
+++ b/www/admin/banner-modify.php
@@ -29,10 +29,11 @@ phpAds_registerGlobal('bannerid', 'campaignid', 'clientid', 'returnurl', 'duplic
 
 
 // Security check
 OA_Permission::enforceAccount(OA_ACCOUNT_MANAGER);
 
+OA_Permission::checkSessionToken();
 
 /*-------------------------------------------------------*/
 /* Main code                                             */
 /*-------------------------------------------------------*/
 
diff --git a/www/admin/banner-swf.php b/www/admin/banner-swf.php
index 0f6a101f66..7e671a5d21 100644
--- a/www/admin/banner-swf.php
+++ b/www/admin/banner-swf.php
@@ -33,10 +33,12 @@ phpAds_registerGlobalUnslashed('convert', 'cancel', 'compress', 'convert_links',
 OA_Permission::enforceAccount(OA_ACCOUNT_MANAGER, OA_ACCOUNT_ADVERTISER);
 OA_Permission::enforceAccessToObject('clients',   $clientid);
 OA_Permission::enforceAccessToObject('campaigns', $campaignid);
 OA_Permission::enforceAccessToObject('banners',   $bannerid);
 
+    OA_Permission::checkSessionToken();
+
 if (OA_Permission::isAccount(OA_ACCOUNT_ADVERTISER)) {
     OA_Permission::enforceAllowed(OA_PERM_BANNER_EDIT);
 }
 
 /*-------------------------------------------------------*/
diff --git a/www/admin/banner-zone.php b/www/admin/banner-zone.php
index 8ab74c2f90..6fc97ec4eb 100644
--- a/www/admin/banner-zone.php
+++ b/www/admin/banner-zone.php
@@ -29,10 +29,12 @@ require_once MAX_PATH . '/lib/OA/Maintenance/Priority.php';
 OA_Permission::enforceAccount(OA_ACCOUNT_MANAGER);
 OA_Permission::enforceAccessToObject('clients',   $clientid);
 OA_Permission::enforceAccessToObject('campaigns', $campaignid);
 OA_Permission::enforceAccessToObject('banners',   $bannerid);
 
+OA_Permission::checkSessionToken();
+
 /*-------------------------------------------------------*/
 /* Store preferences									 */
 /*-------------------------------------------------------*/
 $session['prefs']['inventory_entities'][OA_Permission::getEntityId()]['clientid'] = $clientid;
 $session['prefs']['inventory_entities'][OA_Permission::getEntityId()]['campaignid'][$clientid] = $campaignid;
diff --git a/www/admin/tracker-modify.php b/www/admin/tracker-modify.php
index faa4834e86..41e10c9fd8 100644
--- a/www/admin/tracker-modify.php
+++ b/www/admin/tracker-modify.php
@@ -36,10 +36,12 @@ OA_Permission::enforceAccessToObject('trackers', $trackerid);
 /* Main code                                             */
 /*-------------------------------------------------------*/
 
 if (!empty($trackerid))
 {
+    OA_Permission::checkSessionToken();
+    
     if (!empty($moveto))
     {
         // Delete any campaign-tracker links
         $doCampaign_trackers = OA_Dal::factoryDO('campaigns_trackers');
         $doCampaign_trackers->trackerid = $trackerid;

Vulnerable code:     if (isset($bannerid) && $bannerid != '') {
        // Update banner
        $doBanners = OA_Dal::factoryDO('banners');
        $doBanners->get($bannerid);
        $doBanners->prepend = $prepend;
        $doBanners->append  = $append;
        $doBanners->update();

        // Queue confirmation message
        $translation = new OX_Translation();
        $translated_message = $translation->translate($GLOBALS['strBannerAdvancedHasBeenUpdated'], array(
            MAX::constructURL(MAX_URL_ADMIN, 'banner-edit.php?clientid=' .  $clientid . '&campaignid=' . $campaignid . '&bannerid=' . $bannerid),
            htmlspecialchars($doBanners->description)
        ));
        OA_Admin_UI::queueMessage($translated_message, 'local', 'confirm', 0);
    }
Fixed code:     OA_Permission::checkSessionToken();

OA_Permission::checkSessionToken();

    OA_Permission::checkSessionToken();

    // Update banner
    $doBanners = OA_Dal::factoryDO('banners');
    $doBanners->get($bannerid);
    $doBanners->prepend = $prepend;
    $doBanners->append  = $append;
    $doBanners->update();

    // Queue confirmation message
    $translation = new OX_Translation();
    $translated_message = $translation->translate($GLOBALS['strBannerAdvancedHasBeenUpdated'], array(
        MAX::constructURL(MAX_URL_ADMIN, 'banner-edit.php?clientid=' .  $clientid . '&campaignid=' . $campaignid . '&bannerid=' . $bannerid),
        htmlspecialchars($doBanners->description)
    ));

    OA_Admin_UI::queueMessage($translated_message, 'local', 'confirm', 0);

    exit;
OA_Permission::checkSessionToken();
    OA_Permission::checkSessionToken();

OA_Permission::checkSessionToken();

    OA_Permission::checkSessionToken();