IDOR in Banner Deletion
When Parent Validation Isn't Enough

I found an IDOR vulnerability in Revive Adserver's banner deletion endpoint - now tracked as CVE-2025-52670. It's textbook CWE-639: Authorization Bypass Through User-Controlled Key.

The /www/admin/delete-banner.php endpoint handles bulk banner deletion. It does three things right: verifies you're a manager, checks you have delete permissions, and validates your CSRF token. But it makes one critical mistake.

The endpoint checks if you have access to the clientid and campaignid parameters. These pass. Then it takes whatever bannerid values you supply and deletes them. No ownership check. No validation that those banners belong to the campaign. Just... delete.

This is the classic "validate the parent, forget the child" pattern. And it means any manager can delete any other manager's banners.

Visual Walkthrough

▶ Play↺ ResetStep 1 of 4

1

2

3

4

Loading...

Login as Manager A

Attacker authenticates with valid manager credentials.

What the Victim Sees

Nothing. Manager B logs in the next day and their banner is just... gone. The audit log shows a deletion, but from a session that looks like routine activity. Unless they check timestamps carefully, they won't know what happened.

The Vulnerable Code

Here's the authorization block in delete-banner.php:

// Security checks - Lines 28-34
OA_Permission::enforceAccount(OA_ACCOUNT_MANAGER);
OA_Permission::enforceAccountPermission(OA_ACCOUNT_MANAGER, OA_PERM_MANAGER_DELETE);
OA_Permission::enforceAccessToObject('clients', $clientid);      // Validates client
OA_Permission::enforceAccessToObject('campaigns', $campaignid);  // Validates campaign
// MISSING: OA_Permission::enforceAccessToObject('banners', $bannerid);

OA_Permission::checkSessionToken();  // CSRF check

And here's the deletion loop:

// Deletion loop - Lines 40-48
if (!empty($bannerid)) {
  $ids = explode(',', $bannerid);
  foreach ($ids as $bannerid) {
      $doBanners = OA_Dal::factoryDO('banners');
      $doBanners->bannerid = $bannerid;  // Accepts ANY ID
      if ($doBanners->get($bannerid)) {
          $aBanner = $doBanners->toArray();
      }
      $doBanners->delete();  // Deletes without ownership check
  }
}

How It Should Look

Compare with campaign-delete.php, which handles this correctly:

// campaign-delete.php - Correct pattern
foreach ($ids as $campaignid) {
  OA_Permission::enforceAccessToObject('campaigns', $campaignid);  // Check INSIDE loop
  $doCampaigns = OA_Dal::factoryDO('campaigns');
  $doCampaigns->delete();
}

The campaign deletion validates ownership inside the loop for each ID. The banner deletion doesn't.

Attack Characteristics

This isn't a sophisticated attack. That's what makes it dangerous.

Low privilege required — Any manager account works. No special tools — Just a browser and URL manipulation. Stealthy — Looks like legitimate deletion in logs. Scalable — Can automate to delete hundreds of banners. Cross-tenant — Manager in Agency X can attack Manager in Agency Y.

The Fix

One line. That's all it takes:

if (!empty($bannerid)) {
  $ids = explode(',', $bannerid);
  foreach ($ids as $bannerid) {
      // ADD THIS LINE:
      OA_Permission::enforceAccessToObject('banners', $bannerid);

      $doBanners = OA_Dal::factoryDO('banners');
      $doBanners->bannerid = $bannerid;
      if ($doBanners->get($bannerid)) {
          $aBanner = $doBanners->toArray();
      }
      $doBanners->delete();
  }
}