diff --git a/app/Controllers/Config.php b/app/Controllers/Config.php index f1d63ed4c..738c50b3d 100644 --- a/app/Controllers/Config.php +++ b/app/Controllers/Config.php @@ -11,6 +11,7 @@ use App\Models\Appconfig; use App\Models\Attribute; use App\Models\Customer_rewards; use App\Models\Dinner_table; +use App\Models\Item; use App\Models\Module; use App\Models\Enums\Rounding_mode; use App\Models\Stock_location; @@ -385,9 +386,9 @@ class Config extends Secure_Controller 'gcaptcha_enable' => $this->request->getPost('gcaptcha_enable') != null, 'gcaptcha_secret_key' => $this->request->getPost('gcaptcha_secret_key'), 'gcaptcha_site_key' => $this->request->getPost('gcaptcha_site_key'), - 'suggestions_first_column' => $this->request->getPost('suggestions_first_column'), - 'suggestions_second_column' => $this->request->getPost('suggestions_second_column'), - 'suggestions_third_column' => $this->request->getPost('suggestions_third_column'), + 'suggestions_first_column' => $this->validateSuggestionsColumn($this->request->getPost('suggestions_first_column'), 'first'), + 'suggestions_second_column' => $this->validateSuggestionsColumn($this->request->getPost('suggestions_second_column'), 'other'), + 'suggestions_third_column' => $this->validateSuggestionsColumn($this->request->getPost('suggestions_third_column'), 'other'), 'giftcard_number' => $this->request->getPost('giftcard_number'), 'derive_sale_quantity' => $this->request->getPost('derive_sale_quantity') != null, 'multi_pack_enabled' => $this->request->getPost('multi_pack_enabled') != null, @@ -976,4 +977,26 @@ class Config extends Secure_Controller return $this->response->setJSON(['success' => $success]); } + + /** + * Validates suggestions column configuration to prevent SQL injection. + * + * @param mixed $column The column value from POST + * @param string $fieldType Either 'first' or 'other' to determine default fallback + * @return string Validated column name + */ + private function validateSuggestionsColumn(mixed $column, string $fieldType): string + { + if (!is_string($column)) { + return $fieldType === 'first' ? 'name' : ''; + } + + $allowed = $fieldType === 'first' + ? Item::ALLOWED_SUGGESTIONS_COLUMNS + : Item::ALLOWED_SUGGESTIONS_COLUMNS_WITH_EMPTY; + + $fallback = $fieldType === 'first' ? 'name' : ''; + + return in_array($column, $allowed, true) ? $column : $fallback; + } } diff --git a/app/Models/Item.php b/app/Models/Item.php index 0d21edaaa..08f41bab7 100644 --- a/app/Models/Item.php +++ b/app/Models/Item.php @@ -16,6 +16,10 @@ use stdClass; */ class Item extends Model { + + public const ALLOWED_SUGGESTIONS_COLUMNS = ['name', 'item_number', 'description', 'cost_price', 'unit_price']; + public const ALLOWED_SUGGESTIONS_COLUMNS_WITH_EMPTY = ['', 'name', 'item_number', 'description', 'cost_price', 'unit_price']; + public const ALLOWED_BULK_EDIT_FIELDS = [ 'name', 'category', @@ -27,7 +31,6 @@ class Item extends Model 'allow_alt_description', 'is_serialized' ]; - protected $table = 'items'; protected $primaryKey = 'item_id'; protected $useAutoIncrement = true; @@ -544,13 +547,17 @@ class Item extends Model public function get_search_suggestion_format(?string $seed = null): string { $config = config(OSPOS::class)->settings; - $seed .= ',' . $config['suggestions_first_column']; + + $suggestionsFirstColumn = $this->suggestionColumnIsAllowed($config['suggestions_first_column']) + ? $config['suggestions_first_column'] + : 'name'; + $seed .= ',' . $suggestionsFirstColumn; - if ($config['suggestions_second_column'] !== '') { + if ($config['suggestions_second_column'] !== '' && $this->suggestionColumnIsAllowed($config['suggestions_second_column'])) { $seed .= ',' . $config['suggestions_second_column']; } - if ($config['suggestions_third_column'] !== '') { + if ($config['suggestions_third_column'] !== '' && $this->suggestionColumnIsAllowed($config['suggestions_third_column'])) { $seed .= ',' . $config['suggestions_third_column']; } @@ -566,9 +573,15 @@ class Item extends Model $config = config(OSPOS::class)->settings; $label = ''; - $label1 = $config['suggestions_first_column']; - $label2 = $config['suggestions_second_column']; - $label3 = $config['suggestions_third_column']; + $label1 = $this->suggestionColumnIsAllowed($config['suggestions_first_column']) + ? $config['suggestions_first_column'] + : 'name'; + $label2 = $this->suggestionColumnIsAllowed($config['suggestions_second_column']) + ? $config['suggestions_second_column'] + : ''; + $label3 = $this->suggestionColumnIsAllowed($config['suggestions_third_column']) + ? $config['suggestions_third_column'] + : ''; $this->format_result_numbers($result_row); @@ -592,6 +605,17 @@ class Item extends Model return $label; } + /** + * Validates if a column name is in the allowed suggestions columns. + * + * @param string $columnName + * @return bool + */ + private function suggestionColumnIsAllowed(string $columnName): bool + { + return in_array($columnName, self::ALLOWED_SUGGESTIONS_COLUMNS, true); + } + /** * Converts decimal money values to their correct locale format. * diff --git a/tests/Models/Item_test.php b/tests/Models/Item_test.php new file mode 100644 index 000000000..b58f84e2d --- /dev/null +++ b/tests/Models/Item_test.php @@ -0,0 +1,57 @@ +assertNotContains($malicious, Item::ALLOWED_SUGGESTIONS_COLUMNS, + "Malicious input should not be in allowed columns: $malicious"); + } + } + + public function testSuggestionsColumnValidationAcceptsValidColumns(): void + { + // Valid columns should be preserved + $valid_columns = ['name', 'item_number', 'description', 'cost_price', 'unit_price']; + + foreach ($valid_columns as $column) { + $this->assertContains($column, Item::ALLOWED_SUGGESTIONS_COLUMNS, + "Valid column should be in allowed list: $column"); + } + } + + public function testSuggestionsColumnValidationHandlesEmptyString(): void + { + // Empty string should only be allowed for second and third columns + $this->assertContains('', Item::ALLOWED_SUGGESTIONS_COLUMNS_WITH_EMPTY, + 'Empty string should be allowed for optional columns'); + } + + public function testSuggestionsColumnValidationConstantsAreConsistent(): void + { + // Ensure the constants are properly defined + $this->assertCount(5, Item::ALLOWED_SUGGESTIONS_COLUMNS, + 'Should have exactly 5 allowed columns'); + $this->assertCount(6, Item::ALLOWED_SUGGESTIONS_COLUMNS_WITH_EMPTY, + 'Should have exactly 6 allowed columns including empty'); + + // Ensure empty string is only in WITH_EMPTY variant + $this->assertNotContains('', Item::ALLOWED_SUGGESTIONS_COLUMNS, + 'Empty string should not be in required columns list'); + } +}