Sfoglia il codice sorgente

Harden service unserialize and add security reporting

* Harden service-call storage against object injection

New writes are JSON. Historic PHP-serialized arrays still decode with
allowed_classes disabled and a plain-data check. Alerts and Studio
widgets no longer call unserialize() directly. SECURITY.md adds GHSA
private reporting.

* Wire ACL profile_id decode through the codec helper

getCode() now calls decodeAclProfileId() so the CVE regression test covers the live path. Codec uses the same BX_DOL die-guard as BxDolService.
Andrey Yasko 2 mesi fa
parent
commit
dce212e70a

+ 6 - 1
SECURITY.md

@@ -10,4 +10,9 @@
 
 ## Reporting a Vulnerability
 
-Please, email us at team@unacms.com to report any security, privacy or performance issues.
+Please report security issues privately. Do not open a public GitHub issue.
+
+1. Prefer [GitHub private vulnerability reporting](https://github.com/unacms/una-g/security/advisories/new) on this repository (creates a draft GHSA the maintainers can publish after a fix).
+2. Or email [team@unacms.com](mailto:team@unacms.com).
+
+We will acknowledge the report and follow up on a fix. Reporters who want credit can be named on the advisory when it is published.

+ 3 - 1
inc/classes/BxDolAlerts.php

@@ -121,7 +121,9 @@ class BxDolAlerts extends BxDol
                     $oHandler = new $aHandler['class']();
                     $oHandler->response($this);
                 } else if(!empty($aHandler['service_call']) && BxDolService::isSerializedService($aHandler['service_call'])) {
-                    $aService = unserialize($aHandler['service_call']);
+                    $aService = BxDolService::decodeServiceCall($aHandler['service_call']);
+                    if (false === $aService)
+                        continue;
 
                     $aParams = array($this);
                     if(isset($aService['params']) && is_array($aService['params']))

+ 14 - 17
inc/classes/BxDolService.php

@@ -72,8 +72,8 @@ class BxDolService extends BxDol
      */
     public static function callSerialized($s, $aMarkers = array(), $sReplaceIn = 'params')
     {
-        $a = @unserialize($s);
-        if (false === $a || !is_array($a))
+        $a = self::decodeServiceCall($s);
+        if (false === $a)
             return '';
 
         if (isset($a[$sReplaceIn]) && $aMarkers)
@@ -134,30 +134,27 @@ class BxDolService extends BxDol
     }
 
     /**
-     * Check if string is serialized array
+     * Check if string is a stored service call (JSON or historic PHP serialize).
      */
     public static function isSerializedService($s)
     {
-        return preg_match('/^a:[\d+]:\{/', $s);
+        return BxDolServiceCallCodec::isEncoded($s);
     }
 
     /**
-     * Serialized service call array
+     * Decode a stored service-call blob. Arrays/scalars only; objects are rejected.
      */
-    public static function getSerializedService($mixedModule, $sMethod, $aParams = array(), $sClass = '')
+    public static function decodeServiceCall($s)
     {
-		$aService = array(
-			'module' => $mixedModule,
-			'method' => $sMethod,
-		);
-
-		if(!empty($aParams))
-			$aService['params'] = $aParams;
-
-		if(!empty($sClass))
-			$aService['class'] = $sClass;
+        return BxDolServiceCallCodec::decode($s);
+    }
 
-		return serialize($aService);
+    /**
+     * Encode a service call for storage. New writes are JSON, not PHP serialize.
+     */
+    public static function getSerializedService($mixedModule, $sMethod, $aParams = array(), $sClass = '')
+    {
+        return BxDolServiceCallCodec::encode($mixedModule, $sMethod, $aParams, $sClass);
     }
 
     protected static function getModule($mixed)

+ 112 - 0
inc/classes/BxDolServiceCallCodec.php

@@ -0,0 +1,112 @@
+<?php defined('BX_DOL') or die('hack attempt');
+/**
+ * Copyright (c) UNA, Inc - https://una.io
+ * MIT License - https://opensource.org/licenses/MIT
+ *
+ * Encode/decode stored service-call blobs.
+ * New writes are JSON. Old PHP-serialized arrays still decode.
+ * Objects are never instantiated (allowed_classes => false + plain-data check).
+ *
+ * Writers of these blobs (inventory, 18 Aug 2026):
+ * - BxDolService::getSerializedService — used by cron transient jobs, background jobs,
+ *   live updates (comments/timeline), search-extended field values, ads form values_src.
+ * - DB tables: sys_cron_jobs.service_call, background jobs.service_call,
+ *   sys_alerts_handlers.service_call, studio widget cnt_notices, live-update service_call.
+ * - Studio/admin paths write alerts + widgets. A lower-privilege INSERT into those
+ *   columns is still "call any module method". This codec blocks object injection,
+ *   not an authorized service call.
+ */
+
+class BxDolServiceCallCodec
+{
+    /**
+     * JSON-encode a service-call array for new writes.
+     */
+    public static function encode($mixedModule, $sMethod, $aParams = array(), $sClass = '')
+    {
+        $aService = array(
+            'module' => $mixedModule,
+            'method' => $sMethod,
+        );
+        if (!empty($aParams))
+            $aService['params'] = $aParams;
+        if (!empty($sClass))
+            $aService['class'] = $sClass;
+
+        return json_encode($aService, JSON_UNESCAPED_UNICODE);
+    }
+
+    /**
+     * True for a PHP-serialized array service call or a JSON object with module+method.
+     */
+    public static function isEncoded($s)
+    {
+        return false !== self::decode($s);
+    }
+
+    /**
+     * Decode a stored blob. Returns an array or false.
+     * Never instantiates PHP objects from the payload.
+     */
+    public static function decode($s)
+    {
+        if (!is_string($s) || $s === '')
+            return false;
+
+        $s = ltrim($s);
+        if ($s === '')
+            return false;
+
+        if ($s[0] === '{') {
+            $a = json_decode($s, true);
+            return self::isServiceArray($a) ? $a : false;
+        }
+
+        // Historic PHP serialize of an array: a:N:{...}
+        if (!preg_match('/^a:\d+:\{/', $s))
+            return false;
+
+        $a = @unserialize($s, array('allowed_classes' => false));
+        return self::isServiceArray($a) ? $a : false;
+    }
+
+    /**
+     * Same decode rules as BxBaseMenuSetAclLevel::getCode() AJAX profile_id.
+     * Numeric string stays a string/number; otherwise json_decode. Never unserialize.
+     */
+    public static function decodeAclProfileId($mixedProfileId)
+    {
+        if (!is_string($mixedProfileId) && !is_numeric($mixedProfileId))
+            return $mixedProfileId;
+
+        $mixedProfileId = urldecode((string)$mixedProfileId);
+        if (!is_numeric($mixedProfileId))
+            $mixedProfileId = json_decode($mixedProfileId, true);
+
+        return $mixedProfileId;
+    }
+
+    protected static function isServiceArray($a)
+    {
+        if (!is_array($a) || !isset($a['module'], $a['method']))
+            return false;
+        if (!is_scalar($a['module']) || !is_scalar($a['method']))
+            return false;
+        return self::isPlainData($a);
+    }
+
+    protected static function isPlainData($v)
+    {
+        if (is_scalar($v) || $v === null)
+            return true;
+        if (!is_array($v))
+            return false;
+        foreach ($v as $k => $item) {
+            if (!is_int($k) && !is_string($k))
+                return false;
+            if (!self::isPlainData($item))
+                return false;
+        }
+        return true;
+    }
+}

+ 4 - 2
studio/classes/BxDolStudioWidgets.php

@@ -63,9 +63,11 @@ class BxDolStudioWidgets extends BxTemplStudioPage
 
     	$aResult = array();
     	foreach($aWidgets as $aWidget) {
+            $sNotices = '';
             if(BxDolService::isSerializedService($aWidget['cnt_notices'])) {
-                $aService = unserialize($aWidget['cnt_notices']);
-                $sNotices = BxDolService::call($aService['module'], $aService['method'], array_merge(array($aWidget), $aService['params']), $aService['class']);
+                $aService = BxDolService::decodeServiceCall($aWidget['cnt_notices']);
+                if ($aService)
+                    $sNotices = BxDolService::call($aService['module'], $aService['method'], array_merge(array($aWidget), isset($aService['params']) ? $aService['params'] : array()), isset($aService['class']) ? $aService['class'] : 'Module');
             }
 
             $aResult[$aWidget['id']] = $sNotices;

+ 1 - 3
template/scripts/BxBaseMenuSetAclLevel.php

@@ -47,9 +47,7 @@ class BxBaseMenuSetAclLevel extends BxTemplMenu
     	$this->mixedProfileId = $mixedProfileId;
 
         if (isset($_SERVER['HTTP_X_REQUESTED_WITH']) && $_SERVER['HTTP_X_REQUESTED_WITH'] == 'XMLHttpRequest' && ($mixedProfileId = bx_get('profile_id', 'post')) && ($iAclLevelId = bx_get('level_id', 'post'))) {
-            $mixedProfileId = urldecode($mixedProfileId);
-            if(!is_numeric($mixedProfileId))
-                $mixedProfileId = json_decode($mixedProfileId, true);
+            $mixedProfileId = BxDolServiceCallCodec::decodeAclProfileId($mixedProfileId);
 
             echoJson($this->setMembership($mixedProfileId, $iAclLevelId, bx_get('duration', 'post') !== false ? (int)bx_get('duration', 'post') : 0, (int)bx_get('card', 'post') > 0));
             exit;

+ 81 - 0
tests/units/inc/classes/BxDolServiceCallCodecTest.php

@@ -0,0 +1,81 @@
+<?php
+
+/**
+ * Service-call codec: JSON writes, safe decode of old PHP serialize, no object injection.
+ * Loads the codec file directly so it can run without a full UNA install.
+ */
+class BxDolServiceCallCodecTest extends \PHPUnit\Framework\TestCase
+{
+    public static function setUpBeforeClass(): void
+    {
+        if (!defined('BX_DOL'))
+            define('BX_DOL', 1);
+        if (!class_exists('BxDolServiceCallCodec', false)) {
+            require_once dirname(__DIR__, 4) . '/inc/classes/BxDolServiceCallCodec.php';
+        }
+    }
+
+    public function testEncodeIsJsonWithModuleAndMethod()
+    {
+        $s = BxDolServiceCallCodec::encode('system', 'test', array('a' => 1), 'Module');
+        $this->assertSame('{', $s[0]);
+        $a = json_decode($s, true);
+        $this->assertSame('system', $a['module']);
+        $this->assertSame('test', $a['method']);
+        $this->assertSame(array('a' => 1), $a['params']);
+        $this->assertTrue(BxDolServiceCallCodec::isEncoded($s));
+        $this->assertSame($a, BxDolServiceCallCodec::decode($s));
+    }
+
+    public function testDecodeLegacySerializedArray()
+    {
+        $s = serialize(array(
+            'module' => 'system',
+            'method' => 'test',
+            'params' => array(1, 2),
+        ));
+        $this->assertTrue(BxDolServiceCallCodec::isEncoded($s));
+        $a = BxDolServiceCallCodec::decode($s);
+        $this->assertSame('system', $a['module']);
+        $this->assertSame('test', $a['method']);
+        $this->assertSame(array(1, 2), $a['params']);
+    }
+
+    public function testRejectSerializedObject()
+    {
+        $s = serialize(new stdClass());
+        $this->assertFalse(BxDolServiceCallCodec::isEncoded($s));
+        $this->assertFalse(BxDolServiceCallCodec::decode($s));
+    }
+
+    public function testRejectSerializedArrayContainingObject()
+    {
+        $s = serialize(array(
+            'module' => 'system',
+            'method' => 'test',
+            'params' => array(new stdClass()),
+        ));
+        $this->assertFalse(BxDolServiceCallCodec::decode($s));
+    }
+
+    public function testRejectGarbageAndEmpty()
+    {
+        $this->assertFalse(BxDolServiceCallCodec::decode(''));
+        $this->assertFalse(BxDolServiceCallCodec::decode('not a blob'));
+        $this->assertFalse(BxDolServiceCallCodec::decode('{"foo":1}'));
+        $this->assertFalse(BxDolServiceCallCodec::decode(null));
+    }
+
+    /**
+     * Lock CVE-2025-32101 / CVE-2025-66571: posted profile_id is json_decode, never unserialize.
+     */
+    public function testAclPostedProfileIdDoesNotUnserialize()
+    {
+        $this->assertSame('12', BxDolServiceCallCodec::decodeAclProfileId('12'));
+        $this->assertSame(array(1, 2), BxDolServiceCallCodec::decodeAclProfileId(urlencode(json_encode(array(1, 2)))));
+
+        $serializedObject = serialize(new stdClass());
+        $this->assertNull(BxDolServiceCallCodec::decodeAclProfileId($serializedObject));
+        $this->assertNull(BxDolServiceCallCodec::decodeAclProfileId(urlencode($serializedObject)));
+    }
+}