From bdba55ddc567c1ceb36e7df4fe855d75d87806c0 Mon Sep 17 00:00:00 2001 From: Stanislas Kita Date: Tue, 2 Jun 2026 16:04:16 +0200 Subject: [PATCH 1/8] Fix(XML): Sanitize invalid char --- CHANGELOG.md | 4 ++++ inc/sccm.class.php | 31 +++++++++++++++++++++++-------- 2 files changed, 27 insertions(+), 8 deletions(-) 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..ed987ca 100644 --- a/inc/sccm.class.php +++ b/inc/sccm.class.php @@ -92,6 +92,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 = self::sanitizeRow($tab); $tab['MD-SystemName'] = strtoupper((string) $tab['MD-SystemName']); $this->devices[] = $tab; $i++; @@ -118,7 +119,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[] = self::sanitizeRow($tab); $i++; } @@ -145,7 +146,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[] = self::sanitizeRow($tab); $i++; } @@ -177,7 +178,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[] = self::sanitizeRow($tab); $i++; } @@ -207,7 +208,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[] = self::sanitizeRow($tab); $i++; } @@ -233,7 +234,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[] = self::sanitizeRow($tab); $i++; } @@ -255,7 +256,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[] = self::sanitizeRow($tab); $i++; } @@ -286,7 +287,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[] = self::sanitizeRow($tab); $i++; } @@ -311,13 +312,27 @@ 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[] = self::sanitizeRow($tab); $i++; } return $data; } + private static function sanitizeRow(array $row): array + { + return array_map(static function ($v) { + if (!is_string($v)) { + return $v; + } + if (($pos = strpos($v, "\0")) !== false) { + $v = substr($v, 0, $pos); + } + $v = mb_scrub($v, 'UTF-8'); + return preg_replace('/[\x{FFFE}\x{FFFF}]/u', '', $v) ?? ''; + }, $row); + } + public static function install(): void { $cron = new CronTask(); From f2673f5d16a0b1207f93cd840a67381163e81a47 Mon Sep 17 00:00:00 2001 From: Stanislas Kita Date: Wed, 3 Jun 2026 10:26:14 +0200 Subject: [PATCH 2/8] fix CI --- inc/sccm.class.php | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/inc/sccm.class.php b/inc/sccm.class.php index ed987ca..9c2b21c 100644 --- a/inc/sccm.class.php +++ b/inc/sccm.class.php @@ -40,6 +40,7 @@ use function Safe\simplexml_load_file; use function Safe\sqlsrv_fetch_array; use function Safe\mkdir; +use function Safe\preg_replace; class PluginSccmSccm { @@ -92,7 +93,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 = self::sanitizeRow($tab); + $tab = $this->sanitizeRow($tab); $tab['MD-SystemName'] = strtoupper((string) $tab['MD-SystemName']); $this->devices[] = $tab; $i++; @@ -119,7 +120,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[] = self::sanitizeRow($tab); + $data[] = $this->sanitizeRow($tab); $i++; } @@ -146,7 +147,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[] = self::sanitizeRow($tab); + $data[] = $this->sanitizeRow($tab); $i++; } @@ -178,7 +179,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[] = self::sanitizeRow($tab); + $data[] = $this->sanitizeRow($tab); $i++; } @@ -208,7 +209,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[] = self::sanitizeRow($tab); + $data[] = $this->sanitizeRow($tab); $i++; } @@ -234,7 +235,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[] = self::sanitizeRow($tab); + $data[] = $this->sanitizeRow($tab); $i++; } @@ -256,7 +257,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[] = self::sanitizeRow($tab); + $data[] = $this->sanitizeRow($tab); $i++; } @@ -287,7 +288,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[] = self::sanitizeRow($tab); + $data[] = $this->sanitizeRow($tab); $i++; } @@ -312,22 +313,24 @@ public function getMedias(PluginSccmSccmdb $sccm_db, $deviceid, $limit = 9999999 $data = []; $i = 0; while (($tab = sqlsrv_fetch_array($result, SQLSRV_FETCH_ASSOC)) && $i < $limit) { - $data[] = self::sanitizeRow($tab); + $data[] = $this->sanitizeRow($tab); $i++; } return $data; } - private static function sanitizeRow(array $row): array + private function sanitizeRow(array $row): array { return array_map(static function ($v) { if (!is_string($v)) { return $v; } + if (($pos = strpos($v, "\0")) !== false) { $v = substr($v, 0, $pos); } + $v = mb_scrub($v, 'UTF-8'); return preg_replace('/[\x{FFFE}\x{FFFF}]/u', '', $v) ?? ''; }, $row); From b2ebecf6b4fd69b6f4a1e9db8c443da8f6e90d97 Mon Sep 17 00:00:00 2001 From: Stanislas Kita Date: Wed, 3 Jun 2026 10:52:34 +0200 Subject: [PATCH 3/8] better fix --- inc/sccm.class.php | 16 ++++---- tests/PluginSccmSccmTest.php | 79 ++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/inc/sccm.class.php b/inc/sccm.class.php index 9c2b21c..5ce7cd6 100644 --- a/inc/sccm.class.php +++ b/inc/sccm.class.php @@ -326,13 +326,15 @@ private function sanitizeRow(array $row): array if (!is_string($v)) { return $v; } - - if (($pos = strpos($v, "\0")) !== false) { - $v = substr($v, 0, $pos); - } - - $v = mb_scrub($v, 'UTF-8'); - return preg_replace('/[\x{FFFE}\x{FFFF}]/u', '', $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); } diff --git a/tests/PluginSccmSccmTest.php b/tests/PluginSccmSccmTest.php index 2cb8ffa..013ab10 100644 --- a/tests/PluginSccmSccmTest.php +++ b/tests/PluginSccmSccmTest.php @@ -30,9 +30,88 @@ */ use Glpi\Tests\GLPITestCase; +use PHPUnit\Framework\Attributes\DataProvider; class PluginSccmSccmTest extends GLPITestCase { + private function callSanitizeRow(array $row): array + { + $sccm = $this->getMockBuilder(PluginSccmSccm::class) + ->disableOriginalConstructor() + ->onlyMethods([]) + ->getMock(); + + 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", 'Inte']; + 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 = tempnam(sys_get_temp_dir(), 'sccm_test_') . '.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('PublisherWithControlChars', (string) $loaded->PUBLISHER); + } + public function testGetcomputerQueryBaseStructure(): void { $query = PluginSccmSccm::getcomputerQuery(); From f36e4250b5e4bf3036ce2497f27cc35b35f8862c Mon Sep 17 00:00:00 2001 From: Stanislas Kita Date: Wed, 3 Jun 2026 10:57:25 +0200 Subject: [PATCH 4/8] fix CI --- inc/sccm.class.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/inc/sccm.class.php b/inc/sccm.class.php index 5ce7cd6..83611c6 100644 --- a/inc/sccm.class.php +++ b/inc/sccm.class.php @@ -41,6 +41,7 @@ use function Safe\sqlsrv_fetch_array; use function Safe\mkdir; use function Safe\preg_replace; +use function Safe\mb_convert_encoding; class PluginSccmSccm { @@ -326,6 +327,7 @@ private function sanitizeRow(array $row): array 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: From c0fa800f9722e2ea72091f4238e62a6e0abac2ed Mon Sep 17 00:00:00 2001 From: Stanislas Kita Date: Wed, 3 Jun 2026 10:58:32 +0200 Subject: [PATCH 5/8] fix CI --- tests/PluginSccmSccmTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PluginSccmSccmTest.php b/tests/PluginSccmSccmTest.php index 013ab10..070ffa2 100644 --- a/tests/PluginSccmSccmTest.php +++ b/tests/PluginSccmSccmTest.php @@ -109,7 +109,7 @@ public function testSanitizeRowProducesLoadableXml(): void $this->assertNotFalse($loaded, 'XML must be loadable after sanitization'); $this->assertEmpty($errors, 'No libxml errors expected'); - $this->assertSame('PublisherWithControlChars', (string) $loaded->PUBLISHER); + $this->assertSame('PublisherWithControlCharstrailing', (string) $loaded->PUBLISHER); } public function testGetcomputerQueryBaseStructure(): void From df815fc3e64ca67f930ec238158b4284ecb0c0ca Mon Sep 17 00:00:00 2001 From: Stanislas Kita Date: Wed, 3 Jun 2026 11:22:46 +0200 Subject: [PATCH 6/8] fix CI --- tests/PluginSccmSccmTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PluginSccmSccmTest.php b/tests/PluginSccmSccmTest.php index 070ffa2..00f8824 100644 --- a/tests/PluginSccmSccmTest.php +++ b/tests/PluginSccmSccmTest.php @@ -74,7 +74,7 @@ public function testSanitizeRowStripsIllegalXmlChars(string $input, string $expe public static function illegalXmlCharProvider(): iterable { yield 'null byte NCHAR padding' => ["Intel\x00\x00\x00", 'Intel']; - yield 'null byte mid-string' => ["Inte\x00l", 'Inte']; + 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']; From 96ef3b739e81893f8a16ee1d77a5b502ee4b74dc Mon Sep 17 00:00:00 2001 From: Stanislas Date: Wed, 3 Jun 2026 14:21:21 +0200 Subject: [PATCH 7/8] Update tests/PluginSccmSccmTest.php Co-authored-by: Romain B. <8530352+Rom1-B@users.noreply.github.com> --- tests/PluginSccmSccmTest.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/PluginSccmSccmTest.php b/tests/PluginSccmSccmTest.php index 00f8824..6cc53a3 100644 --- a/tests/PluginSccmSccmTest.php +++ b/tests/PluginSccmSccmTest.php @@ -36,10 +36,7 @@ class PluginSccmSccmTest extends GLPITestCase { private function callSanitizeRow(array $row): array { - $sccm = $this->getMockBuilder(PluginSccmSccm::class) - ->disableOriginalConstructor() - ->onlyMethods([]) - ->getMock(); + $sccm = (new \ReflectionClass(PluginSccmSccm::class))->newInstanceWithoutConstructor(); return (new \ReflectionMethod(PluginSccmSccm::class, 'sanitizeRow'))->invoke($sccm, $row); } From 39bd9c34461d34ab3ce2f025b4c8f0eab64e6630 Mon Sep 17 00:00:00 2001 From: Stanislas Date: Wed, 3 Jun 2026 14:21:28 +0200 Subject: [PATCH 8/8] Update tests/PluginSccmSccmTest.php Co-authored-by: Romain B. <8530352+Rom1-B@users.noreply.github.com> --- tests/PluginSccmSccmTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PluginSccmSccmTest.php b/tests/PluginSccmSccmTest.php index 6cc53a3..2c10746 100644 --- a/tests/PluginSccmSccmTest.php +++ b/tests/PluginSccmSccmTest.php @@ -95,7 +95,7 @@ public function testSanitizeRowProducesLoadableXml(): void "", ); $sxml->PUBLISHER[0] = $result['pub']; - $file = tempnam(sys_get_temp_dir(), 'sccm_test_') . '.ocs'; + $file = sys_get_temp_dir() . '/sccm_test_' . uniqid() . '.ocs'; $sxml->asXML($file); libxml_use_internal_errors(true);