Fix h1 report 175626

Deserialization of Untrusted Data --------------------------------- HackerOne user Nicolas Grégoire - Agarri has reported that Revive Adserver was unserializing untrusted data submitted via cookies in the delivery scripts. An attacker could use such vector to either perform generic RCE attacks (e.g. when a vulnerable PHP version is being used) or, potentially, application-specific attacks. CWE-ID: CWE-502 CVSSv3 Vector: CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H/E:U/RL:O/RC:C CVSSv3 Base Score: 9.8 CVSSv3 Temporal Score: 8.5
revive-adserver · Jan 30, 2017 · 05bb9f4 · 05bb9f4
1 parent 83a12b2
commit 05bb9f4
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 19 deletions.
diff --git a/lib/max/Delivery/querystring.php b/lib/max/Delivery/querystring.php
@@ -71,10 +71,12 @@ function MAX_querystringConvertParams()
         $n = isset($aGet[$conf['var']['n']]) ? $aGet[$conf['var']['n']] : '';
     }
     if (!empty($n) && !empty($_COOKIE[$conf['var']['vars']][$n])) {
-        $aVars = unserialize(stripslashes($_COOKIE[$conf['var']['vars']][$n]));
-        foreach ($aVars as $name => $value) {
-            if (!isset($_GET[$name])) {
-                $aGet[$name] = $value;
+        $aVars = json_decode($_COOKIE[$conf['var']['vars']][$n], true);
+        if (is_array($aVars)) {
+            foreach ($aVars as $name => $value) {
+                if (!isset($_GET[$name])) {
+                    $aGet[$name] = $value;
+                }
             }
         }
     }

diff --git a/www/delivery_dev/afr.php b/www/delivery_dev/afr.php
@@ -32,20 +32,24 @@
 $banner = MAX_adSelect($what, $campaignid, $target, $source, $withtext, $charset, $context, true, $ct0, $loc, $referer);
 
 // Send cookie if needed
-if (!empty($banner['html']) && !empty($n)) {
-    // Send bannerid headers
-    $cookie = array();
-    $cookie[$conf['var']['adId']] = $banner['bannerid'];
-    // Send zoneid headers
-    if ($zoneid != 0) {
-        $cookie[$conf['var']['zoneId']] = $zoneid;
+if (!empty($n)) {
+    if (!empty($banner['html'])) {
+        // Send bannerid headers
+        $cookie = array();
+        $cookie[$conf['var']['adId']] = $banner['bannerid'];
+        // Send zoneid headers
+        if ($zoneid != 0) {
+            $cookie[$conf['var']['zoneId']] = $zoneid;
+        }
+        // Send source headers
+        if (!empty($source)) {
+            $cookie[$conf['var']['channel']] = $source;
+        }
+        // Set the cookie
+        MAX_cookieAdd($conf['var']['vars'] . "[$n]", json_encode($cookie, JSON_UNESCAPED_SLASHES));
+    } else {
+        MAX_cookieUnset($conf['var']['vars'] . "[$n]");
     }
-    // Send source headers
-    if (!empty($source)) {
-        $cookie[$conf['var']['channel']] = $source;
-    }
-    // Set the cookie
-    MAX_cookieAdd($conf['var']['vars'] . "[$n]", serialize($cookie));
 }
 
 MAX_cookieFlush();

diff --git a/www/delivery_dev/avw.php b/www/delivery_dev/avw.php
@@ -84,7 +84,7 @@
         MAX_Delivery_log_logAdImpression($row['bannerid'], $zoneid);
     }
     // Redirect to the banner
-    MAX_cookieAdd($conf['var']['vars'] . "[$n]", serialize($cookie));
+    MAX_cookieAdd($conf['var']['vars'] . "[$n]", json_encode($cookie, JSON_UNESCAPED_SLASHES));
     MAX_cookieFlush();
     if ($row['bannerid'] == '') {
        if ($row['default_banner_image_url'] != '') {
@@ -98,7 +98,7 @@
         MAX_redirect($creativeURL);
     }
 } else {
-	MAX_cookieAdd($conf['var']['vars'] . "[$n]", 'DEFAULT');
+	MAX_cookieUnset($conf['var']['vars'] . "[$n]");
 	MAX_cookieFlush();
 	// Show 1x1 Gif, to ensure not broken image icon is shown.
 	MAX_commonDisplay1x1();