diff --git a/CHANGELOG.md b/CHANGELOG.md index c6e1c30..882b794 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [UNRELEASED] +### Fixed + +- Sanitize invalid characters from SCCM + ## [2.6.0] - 2026-05-26 ### Added diff --git a/inc/sccm.class.php b/inc/sccm.class.php index d86e9b5..83611c6 100644 --- a/inc/sccm.class.php +++ b/inc/sccm.class.php @@ -40,6 +40,8 @@ use function Safe\simplexml_load_file; use function Safe\sqlsrv_fetch_array; use function Safe\mkdir; +use function Safe\preg_replace; +use function Safe\mb_convert_encoding; class PluginSccmSccm { @@ -92,6 +94,7 @@ public function getDevices(int $config_id, $where = 0, $limit = 99999999): void $this->devices = []; while (($tab = sqlsrv_fetch_array($result, SQLSRV_FETCH_ASSOC)) && $i < $limit) { + $tab = $this->sanitizeRow($tab); $tab['MD-SystemName'] = strtoupper((string) $tab['MD-SystemName']); $this->devices[] = $tab; $i++; @@ -118,7 +121,7 @@ public function getDatas(PluginSccmSccmdb $sccm_db, $type, $deviceid, $limit = 9 $data = []; $i = 0; while (($tab = sqlsrv_fetch_array($result, SQLSRV_FETCH_ASSOC)) && $i < $limit) { - $data[] = $tab; + $data[] = $this->sanitizeRow($tab); $i++; } @@ -145,7 +148,7 @@ public function getNetwork(PluginSccmSccmdb $sccm_db, $deviceid, $limit = 999999 $data = []; $i = 0; while (($tab = sqlsrv_fetch_array($result, SQLSRV_FETCH_ASSOC)) && $i < $limit) { - $data[] = $tab; + $data[] = $this->sanitizeRow($tab); $i++; } @@ -177,7 +180,7 @@ public function getSoftware(PluginSccmSccmdb $sccm_db, $deviceid, $limit = 99999 $data = []; $i = 0; while (($tab = sqlsrv_fetch_array($result, SQLSRV_FETCH_ASSOC)) && $i < $limit) { - $data[] = $tab; + $data[] = $this->sanitizeRow($tab); $i++; } @@ -207,7 +210,7 @@ public function getMemories(PluginSccmSccmdb $sccm_db, $deviceid, $limit = 99999 $data = []; $i = 0; while (($tab = sqlsrv_fetch_array($result, SQLSRV_FETCH_ASSOC)) && $i < $limit) { - $data[] = $tab; + $data[] = $this->sanitizeRow($tab); $i++; } @@ -233,7 +236,7 @@ public function getVideos(PluginSccmSccmdb $sccm_db, $deviceid, $limit = 9999999 $data = []; $i = 0; while (($tab = sqlsrv_fetch_array($result, SQLSRV_FETCH_ASSOC)) && $i < $limit) { - $data[] = $tab; + $data[] = $this->sanitizeRow($tab); $i++; } @@ -255,7 +258,7 @@ public function getSounds(PluginSccmSccmdb $sccm_db, $deviceid, $limit = 9999999 $data = []; $i = 0; while (($tab = sqlsrv_fetch_array($result, SQLSRV_FETCH_ASSOC)) && $i < $limit) { - $data[] = $tab; + $data[] = $this->sanitizeRow($tab); $i++; } @@ -286,7 +289,7 @@ public function getStorages(PluginSccmSccmdb $sccm_db, $deviceid, $limit = 99999 $data = []; $i = 0; while (($tab = sqlsrv_fetch_array($result, SQLSRV_FETCH_ASSOC)) && $i < $limit) { - $data[] = $tab; + $data[] = $this->sanitizeRow($tab); $i++; } @@ -311,13 +314,32 @@ public function getMedias(PluginSccmSccmdb $sccm_db, $deviceid, $limit = 9999999 $data = []; $i = 0; while (($tab = sqlsrv_fetch_array($result, SQLSRV_FETCH_ASSOC)) && $i < $limit) { - $data[] = $tab; + $data[] = $this->sanitizeRow($tab); $i++; } return $data; } + private function sanitizeRow(array $row): array + { + return array_map(static function ($v) { + if (!is_string($v)) { + return $v; + } + + // Fix invalid UTF-8 sequences before applying the XML character filter + $v = mb_convert_encoding($v, 'UTF-8', 'UTF-8'); + // Remove all characters illegal in XML 1.0: + // U+0000–U+0008, U+000B, U+000C, U+000E–U+001F, U+FFFE, U+FFFF + return preg_replace( + '/[^\x{0009}\x{000A}\x{000D}\x{0020}-\x{D7FF}\x{E000}-\x{FFFD}\x{10000}-\x{10FFFF}]/u', + '', + $v, + ) ?? ''; + }, $row); + } + public static function install(): void { $cron = new CronTask(); diff --git a/tests/PluginSccmSccmTest.php b/tests/PluginSccmSccmTest.php index 2cb8ffa..2c10746 100644 --- a/tests/PluginSccmSccmTest.php +++ b/tests/PluginSccmSccmTest.php @@ -30,9 +30,85 @@ */ use Glpi\Tests\GLPITestCase; +use PHPUnit\Framework\Attributes\DataProvider; class PluginSccmSccmTest extends GLPITestCase { + private function callSanitizeRow(array $row): array + { + $sccm = (new \ReflectionClass(PluginSccmSccm::class))->newInstanceWithoutConstructor(); + + return (new \ReflectionMethod(PluginSccmSccm::class, 'sanitizeRow'))->invoke($sccm, $row); + } + + public function testSanitizeRowPreservesCleanString(): void + { + $result = $this->callSanitizeRow(['name' => 'Intel(R) Corporation']); + $this->assertSame('Intel(R) Corporation', $result['name']); + } + + public function testSanitizeRowPreservesNonStringValues(): void + { + $result = $this->callSanitizeRow(['int' => 42, 'null' => null, 'bool' => true]); + $this->assertSame(42, $result['int']); + $this->assertNull($result['null']); + $this->assertTrue($result['bool']); + } + + public function testSanitizeRowPreservesValidXmlWhitespace(): void + { + $result = $this->callSanitizeRow(['ws' => "line1\ttabbed\nline2\rreturn"]); + $this->assertSame("line1\ttabbed\nline2\rreturn", $result['ws']); + } + + #[DataProvider('illegalXmlCharProvider')] + public function testSanitizeRowStripsIllegalXmlChars(string $input, string $expected): void + { + $result = $this->callSanitizeRow(['v' => $input]); + $this->assertSame($expected, $result['v']); + } + + public static function illegalXmlCharProvider(): iterable + { + yield 'null byte NCHAR padding' => ["Intel\x00\x00\x00", 'Intel']; + yield 'null byte mid-string' => ["Inte\x00l", 'Intel']; + yield 'U+FFFE noncharacter' => ["Intel\xEF\xBF\xBE", 'Intel']; + yield 'U+FFFF noncharacter' => ["Intel\xEF\xBF\xBF", 'Intel']; + yield 'U+FFFE and U+FFFF combined' => ["Corp\xEF\xBF\xBE\xEF\xBF\xBF", 'Corp']; + yield 'SOH U+0001' => ["Intel\x01Corp", 'IntelCorp']; + yield 'control chars U+0001–U+0008' => ["A\x01\x02\x03\x04\x05\x06\x07\x08Z", 'AZ']; + yield 'vertical tab U+000B' => ["Intel\x0BCorp", 'IntelCorp']; + yield 'form feed U+000C' => ["Intel\x0CCorp", 'IntelCorp']; + yield 'control chars U+000E–U+001F' => ["A\x0E\x0F\x10\x1FZ", 'AZ']; + yield 'issue #181 production value' => [ + "Intel(R) Corporation\xEF\xBF\xBE\xEF\xBF\xBF", + 'Intel(R) Corporation', + ]; + } + + public function testSanitizeRowProducesLoadableXml(): void + { + $dirty = "Publisher\x01With\x0BControl\xEF\xBF\xBFChars\x00trailing"; + $result = $this->callSanitizeRow(['pub' => $dirty]); + + $sxml = new SimpleXMLElement( + "", + ); + $sxml->PUBLISHER[0] = $result['pub']; + $file = sys_get_temp_dir() . '/sccm_test_' . uniqid() . '.ocs'; + $sxml->asXML($file); + + libxml_use_internal_errors(true); + $loaded = simplexml_load_file($file, 'SimpleXMLElement', LIBXML_NOCDATA); + $errors = libxml_get_errors(); + libxml_clear_errors(); + unlink($file); + + $this->assertNotFalse($loaded, 'XML must be loadable after sanitization'); + $this->assertEmpty($errors, 'No libxml errors expected'); + $this->assertSame('PublisherWithControlCharstrailing', (string) $loaded->PUBLISHER); + } + public function testGetcomputerQueryBaseStructure(): void { $query = PluginSccmSccm::getcomputerQuery();