CVE-2023-38488: Incorrect Authorization (CWE-863)
value: Kirby is a content management system. A vulnerability in versions prior to 3.5.8.3, 3.6.6.3, 3.7.5.2, 3.8.4.1, and 3.9.6 affects all Kirby sites that might have potential attackers in the group of authenticated Panel users or that allow external visitors to update a Kirby content file (e.g. via a contact or comment form). Kirby sites are *not* affected if they don't allow write access for untrusted users or visitors.

A field injection in a content storage implementation is a type of vulnerability that allows attackers with content write access to overwrite content fields that the site developer didn't intend to be modified. In a Kirby site this can be used to alter site content, break site behavior or inject malicious data or code. The exact security risk depends on the field type and usage.

Kirby stores content of the site, of pages, files and users in text files by default. The text files use Kirby's KirbyData format where each field is separated by newlines and a line with four dashes (`----`). When reading a KirbyData file, the affected code first removed the Unicode BOM sequence from the file contents and afterwards split the content into fields by the field separator.

When writing to a KirbyData file, field separators in field data are escaped to prevent user input from interfering with the field structure. However this escaping could be tricked by including a Unicode BOM sequence in a field separator (e.g. `--\xEF\xBB\xBF--`). When writing, this was not detected as a separator, but because the BOM was removed during reading, it could be abused by attackers to inject other field data into content files.

Because each field can only be defined once per content file, this vulnerability only affects fields in the content file that were defined above the vulnerable user-writable field or not at all. Fields that are defined below the vulnerable field override the injected field content and were therefore already protected.

The problem has been patched in Kirby 3.5.8.3, 3.6.6.3, 3.7.5.2, 3.8.4.1, and 3.9.6. In all of the mentioned releases, the maintainers have fixed the affected code to only remove the Unicode BOM sequence at the beginning of the file. This fixes this vulnerability both for newly written as well as for existing content files.
CWE description: The product performs an authorization check when an actor attempts to access a resource or perform an action, but it does not correctly perform the check. This allows attackers to bypass intended access restrictions.
CVSS v3 base score: 8.8
Published: 2023-07-27T15:15Z
Repository: https://github.com/getkirby/kirby
Language: PHP
Files changed: src/Data/Txt.php, tests/Data/TxtTest.php
Commit date: 2023-07-26 21:07:01 +0200
Commit message: Fix KirbyData BOM vulnerability

https://github.com/getkirby/kirby/security/advisories/GHSA-x5mr-p6v4-wp93

Diff: diff --git a/src/Data/Txt.php b/src/Data/Txt.php
index 10b4e64b2..432d25563 100644
--- a/src/Data/Txt.php
+++ b/src/Data/Txt.php
@@ -91,14 +91,18 @@ class Txt extends Handler
 
 		if (is_string($string) === false) {
 			throw new InvalidArgumentException('Invalid TXT data; please pass a string');
 		}
 
-		// remove BOM
-		$string = str_replace("\xEF\xBB\xBF", '', $string);
+		// remove Unicode BOM at the beginning of the file
+		if (Str::startsWith($string, "\xEF\xBB\xBF") === true) {
+			$string = substr($string, 3);
+		}
+
 		// explode all fields by the line separator
 		$fields = preg_split('!\n----\s*\n*!', $string);
+
 		// start the data array
 		$data = [];
 
 		// loop through all fields and add them to the content
 		foreach ($fields as $field) {
diff --git a/tests/Data/TxtTest.php b/tests/Data/TxtTest.php
index 58e4ce2c9..f1a54533f 100644
--- a/tests/Data/TxtTest.php
+++ b/tests/Data/TxtTest.php
@@ -193,10 +193,38 @@ class TxtTest extends TestCase
 
 		$data = Txt::decode(file_get_contents(static::FIXTURES . '/decode.txt'));
 		$this->assertSame($array, $data);
 	}
 
+	/**
+	 * @covers ::decode
+	 */
+	public function testDecodeBom1()
+	{
+		$string = "\xEF\xBB\xBFTitle: title field with BOM \xEF\xBB\xBF\n----\nText: text field";
+		$array  = [
+			'title' => "title field with BOM \xEF\xBB\xBF",
+			'text'  => 'text field'
+		];
+
+		$this->assertSame($array, Txt::decode($string));
+	}
+
+	/**
+	 * @covers ::decode
+	 */
+	public function testDecodeBom2()
+	{
+		$string = "\xEF\xBB\xBFTitle: title field with BOM\n--\xEF\xBB\xBF--\nand more text\n----\nText: text field";
+		$array  = [
+			'title' => "title field with BOM\n--\xEF\xBB\xBF--\nand more text",
+			'text'  => 'text field'
+		];
+
+		$this->assertSame($array, Txt::decode($string));
+	}
+
 	/**
 	 * @covers ::decode
 	 */
 	public function testDecodeInvalid1()
 	{

Vulnerable code: 		// remove BOM
		$string = str_replace("\xEF\xBB\xBF", '', $string);
Fixed code: 		// remove Unicode BOM at the beginning of the file
		if (Str::startsWith($string, "\xEF\xBB\xBF") === true) {
			$string = substr($string, 3);
		}


	/**
	 * @covers ::decode
	 */
	public function testDecodeBom1()
	{
		$string = "\xEF\xBB\xBFTitle: title field with BOM \xEF\xBB\xBF\n----\nText: text field";
		$array  = [
			'title' => "title field with BOM \xEF\xBB\xBF",
			'text'  => 'text field'
		];

		$this->assertSame($array, Txt::decode($string));
	}

	/**
	 * @covers ::decode
	 */
	public function testDecodeBom2()
	{
		$string = "\xEF\xBB\xBFTitle: title field with BOM\n--\xEF\xBB\xBF--\nand more text\n----\nText: text field";
		$array  = [
			'title' => "title field with BOM\n--\xEF\xBB\xBF--\nand more text",
			'text'  => 'text field'
		];

		$this->assertSame($array, Txt::decode($string));
	}