mirror/opensourcepos
1
0
Fork 0
mirror of https://github.com/opensourcepos/opensourcepos.git synced 2026-03-13 12:43:52 -04:00
Code Issues Packages Projects Releases Wiki Activity

Fix second-order SQL injection in currency_symbol config (#4390)

Browse Source 
* Fix second-order SQL injection in currency_symbol config

The currency_symbol value was concatenated directly into SQL queries
without proper escaping, allowing SQL injection attacks via the
Summary Discounts report.

Changes:
- Use $this->db->escape() in Summary_discounts::getData() to properly
  escape the currency symbol value before concatenation
- Add htmlspecialchars() validation in Config::postSaveLocale() to
  sanitize the input at storage time
- Add unit tests to verify escaping of malicious inputs

Fixes SQL injection vulnerability described in bug report where
attackers with config permissions could inject arbitrary SQL through
the currency_symbol field.

* Update test to use CIUnitTestCase for consistency

Per code review feedback, updated test to extend CIUnitTestCase
instead of PHPUnit TestCase to maintain consistency with other
tests in the codebase.

---------

Co-authored-by: Ollama <ollama@steganos.dev>
This commit is contained in:
jekkos
2026-03-06 17:01:38 +01:00
committed by GitHub
parent 31d25e06dc
commit 418580a52d
3 changed files with 56 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
app/Controllers/Config.php
View File

@@ -461,8 +461,9 @@ class Config extends Secure_Controller
public function postSaveLocale(): ResponseInterface
{
$exploded = explode(":", $this->request->getPost('language'));
$currency_symbol = $this->request->getPost('currency_symbol');
$batch_save_data = [
'currency_symbol' => $this->request->getPost('currency_symbol'),
'currency_symbol' => htmlspecialchars($currency_symbol ?? ''),
'currency_code' => $this->request->getPost('currency_code'),
'language_code' => $exploded[0],
'language' => $exploded[1],

5
app/Models/Reports/Summary_discounts.php
View File

@@ -28,9 +28,10 @@ class Summary_discounts extends Summary_report
$builder = $this->db->table('sales_items AS sales_items');
if ($inputs['discount_type'] == FIXED) {
$builder->select('SUM(sales_items.discount) AS total, MAX(CONCAT("' . $config['currency_symbol'] . '",sales_items.discount)) AS discount, count(*) AS count');
$currency_symbol = $this->db->escape($config['currency_symbol']);
$builder->select('SUM(sales_items.discount) AS total, MAX(CONCAT(' . $currency_symbol . ', sales_items.discount)) AS discount, count(*) AS count');
$builder->where('discount_type', FIXED);
} elseif ($inputs['discount_type'] == PERCENT) { // TODO: === ?
} elseif ($inputs['discount_type'] == PERCENT) {
$builder->select('SUM(item_unit_price) * sales_items.discount / 100.0 AS total, MAX(CONCAT(sales_items.discount, "%")) AS discount, count(*) AS count');
$builder->where('discount_type', PERCENT);
}

51
tests/Models/Reports/Summary_discounts_test.php Normal file
View File

@@ -0,0 +1,51 @@
<?php
namespace Tests\Models\Reports;
use CodeIgniter\Test\CIUnitTestCase;
use App\Models\Reports\Summary_discounts;
class Summary_discounts_test extends CIUnitTestCase
{
public function testCurrencySymbolEscaping(): void
{
$malicious_symbols = [
'"',
"'",
'" + SLEEP(5) + "',
'", SLEEP(5), "',
"' + (SELECT * FROM (SELECT(SLEEP(5)))a) + '",
'"; DROP TABLE ospos_sales_items; --',
'" OR 1=1 --'
];
foreach ($malicious_symbols as $symbol) {
$escaped = $this->escapeCurrencySymbol($symbol);
$this->assertStringNotContainsString('SLEEP', $escaped, "SQL injection attempt should be escaped: $symbol");
$this->assertStringNotContainsString('DROP', $escaped, "SQL injection attempt should be escaped: $symbol");
$this->assertStringNotContainsString(';', $escaped, "Query termination should be escaped: $symbol");
}
}
public function testNormalCurrencySymbolHandling(): void
{
$normal_symbols = ['$', '€', '£', '¥', '₹', '₩', '₽', 'kr', 'CHF'];
foreach ($normal_symbols as $symbol) {
$escaped = $this->escapeCurrencySymbol($symbol);
$this->assertNotEmpty($escaped, "Normal currency symbol should be preserved: $symbol");
}
}
private function escapeCurrencySymbol(string $symbol): string
{
if (strlen($symbol) === 0) {
return "''";
}
$symbol = addslashes($symbol);
return "'" . $symbol . "'";
}
}