From dc1accc65a5774fff9a2f22dfab8abdd4d37029b Mon Sep 17 00:00:00 2001 From: jekkos Date: Mon, 28 Sep 2026 08:44:37 +0200 Subject: [PATCH] fix(security): HTML-escape attribute dropdown option labels in items attributes view (#4715) --- app/Views/attributes/item.php | 4 +-- tests/Controllers/ItemsControllerTest.php | 30 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/app/Views/attributes/item.php b/app/Views/attributes/item.php index b807dc10c..75dff9927 100644 --- a/app/Views/attributes/item.php +++ b/app/Views/attributes/item.php @@ -12,7 +12,7 @@
'definition_name', - 'options' => $definition_names, + 'options' => esc($definition_names), 'selected' => -1, 'class' => 'form-control', 'id' => 'definition_name' @@ -45,7 +45,7 @@ $selected_value = $definition_value['selected_value']; echo form_dropdown([ 'name' => "attribute_links[$definition_id]", - 'options' => $definition_value['values'], + 'options' => esc($definition_value['values']), 'selected' => $selected_value, 'class' => 'form-control', 'data-definition-id' => $definition_id diff --git a/tests/Controllers/ItemsControllerTest.php b/tests/Controllers/ItemsControllerTest.php index b584a63ea..895f87868 100644 --- a/tests/Controllers/ItemsControllerTest.php +++ b/tests/Controllers/ItemsControllerTest.php @@ -189,6 +189,36 @@ class ItemsControllerTest extends CIUnitTestCase $this->assertTrue($result['success']); } + /** + * Regression test for GHSA-cm7j-957q-8pgg: an attribute definition whose + * `definition_name` contains HTML must be entity-escaped when rendered in the + * items attributes dropdown, not emitted as a live (executable) tag. + */ + public function testGetAttributesEscapesMaliciousDefinitionName(): void + { + $employeeId = $this->createItemsEmployee(); + $this->loginAsItemsEmployee($employeeId); + + $payload = ''; + + $definitionData = [ + 'definition_name' => $payload, + 'definition_type' => TEXT, + 'definition_flags' => 0, + 'deleted' => 0, + ]; + $this->assertTrue($this->attribute->saveDefinition($definitionData)); + $this->assertNotEmpty($definitionData['definition_id']); + + $response = $this->get('/items/attributes/1'); + $output = (string) $response->getBody(); + + // A live, unescaped tag in the dropdown label is the stored-XSS sink. + $this->assertStringNotContainsString($payload, $output); + // The payload must be present only in entity-escaped form. + $this->assertStringContainsString('<img src=x onerror=alert(1)>', $output); + } + public function testGenerateCsvHeaderBasic(): void { $stockLocations = ['Warehouse'];