Security advisory GHSA-cjfx-qhwm-hf99: FacturaScripts has SQL Injection in API ORDER BY Clause
Package: facturascripts/facturascripts (composer), vulnerable version: v2025.71, fixed in: 2025.81
CWE: CWE-1286
CWE-20
CWE-89
CWE-943
Files changed by the fix:
Core/Base/DataBase/PostgresqlEngine.php
Core/DbQuery.php

Fix diff:
diff --git a/Core/Base/DataBase/PostgresqlEngine.php b/Core/Base/DataBase/PostgresqlEngine.php
index 37129b3191..89cfa33ab3 100644
--- a/Core/Base/DataBase/PostgresqlEngine.php
+++ b/Core/Base/DataBase/PostgresqlEngine.php
@@ -61,7 +61,7 @@ public function beginTransaction($link): bool
 
     public function castInteger($link, $column): string
     {
-        return 'CAST(' . $this->escapeColumn($link, $column) . ' AS unsigned)';
+        return 'CAST(' . $this->escapeColumn($link, $column) . ' AS integer)';
     }
 
     /**
diff --git a/Core/DbQuery.php b/Core/DbQuery.php
index d6eeae2d91..2ce13f49a8 100644
--- a/Core/DbQuery.php
+++ b/Core/DbQuery.php
@@ -1,7 +1,7 @@
 <?php
 /**
  * This file is part of FacturaScripts
- * Copyright (C) 2023-2025 Carlos Garcia Gomez <carlos@facturascripts.com>
+ * Copyright (C) 2023-2026 Carlos Garcia Gomez <carlos@facturascripts.com>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU Lesser General Public License as
@@ -283,26 +283,39 @@ public function offset(int $offset): self
 
     public function orderBy(string $field, string $order = 'ASC'): self
     {
-        // si lleva paréntesis, no escapamos
-        if (strpos($field, '(') !== false && strpos($field, ')') !== false) {
-            $this->orderBy[] = $field . ' ' . $order;
-            return $this;
-        }
+        // validamos que order sea ASC o DESC
+        $order = strtoupper($order) === 'DESC' ? 'DESC' : 'ASC';
 
-        // si contiene espacios, no escapamos
-        if (strpos($field, ' ') !== false) {
-            $this->orderBy[] = $field . ' ' . $order;
-            return $this;
+        // si lleva paréntesis, validamos que sea una expresión permitida
+        if (str_contains($field, '(') && str_contains($field, ')')) {
+            // permitimos LOWER(), UPPER() y CAST()
+            if (preg_match('/^(LOWER|UPPER)\([a-zA-Z0-9_.]+\)$/i', $field) ||
+                preg_match('/^CAST\([a-zA-Z0-9_.]+ AS [a-zA-Z0-9_ ]+\)$/i', $field)) {
+                $this->orderBy[] = $field . ' ' . $order;
+                return $this;
+            }
+            // si no es una expresión permitida, escapamos el campo completo
         }
 
         // si el campo comienza por integer: hacemos el cast a integer
-        if (0 === strpos($field, 'integer:')) {
+        if (str_starts_with($field, 'integer:')) {
             $field = self::db()->castInteger(substr($field, 8));
+            $this->orderBy[] = $field . ' ' . $order;
+            return $this;
         }
 
         // si empieza por lower, hacemos el lower
-        if (0 === strpos($field, 'lower:')) {
+        if (str_starts_with($field, 'lower:')) {
             $field = 'LOWER(' . self::db()->escapeColumn(substr($field, 6)) . ')';
+            $this->orderBy[] = $field . ' ' . $order;
+            return $this;
+        }
+
+        // si empieza por upper, hacemos el upper
+        if (str_starts_with($field, 'upper:')) {
+            $field = 'UPPER(' . self::db()->escapeColumn(substr($field, 6)) . ')';
+            $this->orderBy[] = $field . ' ' . $order;
+            return $this;
         }
 
         $this->orderBy[] = self::db()->escapeColumn($field) . ' ' . $order;
diff --git a/Test/Core/DbQueryTest.php b/Test/Core/DbQueryTest.php
index 40a7169750..54bd762500 100644
--- a/Test/Core/DbQueryTest.php
+++ b/Test/Core/DbQueryTest.php
@@ -1,7 +1,7 @@
 <?php
 /**
  * This file is part of FacturaScripts
- * Copyright (C) 2023-2025 Carlos Garcia Gomez <carlos@facturascripts.com>
+ * Copyright (C) 2023-2026 Carlos Garcia Gomez <carlos@facturascripts.com>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU Lesser General Public License as
@@ -126,6 +126,86 @@ public function testOrderBy(): void
         $this->assertEquals($sql, $query->sql());
     }
 
+    public function testOrderByAdvanced(): void
+    {
+        // si no existe la tabla series, saltamos el test
+        if (false === $this->db()->tableExists('series')) {
+            $this->markTestSkipped('Table series does not exist.');
+        }
+
+        // Test 1: Sanitización de ORDER - DESC en mayúsculas
+        $query1 = DbQuery::table('series')
+            ->orderBy('codserie', 'DESC');
+        $expected1 = 'SELECT * FROM ' . $this->db()->escapeColumn('series')
+            . ' ORDER BY ' . $this->db()->escapeColumn('codserie') . ' DESC';
+        $this->assertEquals($expected1, $query1->sql());
+
+        // Test 2: Sanitización de ORDER - desc en minúsculas
+        $query2 = DbQuery::table('series')
+            ->orderBy('codserie', 'desc');
+        $expected2 = 'SELECT * FROM ' . $this->db()->escapeColumn('series')
+            . ' ORDER BY ' . $this->db()->escapeColumn('codserie') . ' DESC';
+        $this->assertEquals($expected2, $query2->sql());
+
+        // Test 3: Sanitización de ORDER - valor inválido debe convertirse a ASC
+        $query3 = DbQuery::table('series')
+            ->orderBy('codserie', 'INVALID');
+        $expected3 = 'SELECT * FROM ' . $this->db()->escapeColumn('series')
+            . ' ORDER BY ' . $this->db()->escapeColumn('codserie') . ' ASC';
+        $this->assertEquals($expected3, $query3->sql());
+
+        // Test 4: Prefijo lower:
+        $query4 = DbQuery::table('series')
+            ->orderBy('lower:descripcion');
+        $expected4 = 'SELECT * FROM ' . $this->db()->escapeColumn('series')
+            . ' ORDER BY LOWER(' . $this->db()->escapeColumn('descripcion') . ') ASC';
+        $this->assertEquals($expected4, $query4->sql());
+
+        // Test 5: Prefijo upper:
+        $query5 = DbQuery::table('series')
+            ->orderBy('upper:descripcion', 'DESC');
+        $expected5 = 'SELECT * FROM ' . $this->db()->escapeColumn('series')
+            . ' ORDER BY UPPER(' . $this->db()->escapeColumn('descripcion') . ') DESC';
+        $this->assertEquals($expected5, $query5->sql());
+
+        // Test 6: Prefijo integer:
+        $query6 = DbQuery::table('series')
+            ->orderBy('integer:codserie');
+        $expected6 = 'SELECT * FROM ' . $this->db()->escapeColumn('series')
+            . ' ORDER BY ' . $this->db()->castInteger('codserie') . ' ASC';
+        $this->assertEquals($expected6, $query6->sql());
+
+        // Test 7: Expresión permitida LOWER()
+        $query7 = DbQuery::table('series')
+            ->orderBy('LOWER(descripcion)');
+        $expected7 = 'SELECT * FROM ' . $this->db()->escapeColumn('series')
+            . ' ORDER BY LOWER(descripcion) ASC';
+        $this->assertEquals($expected7, $query7->sql());
+
+        // Test 8: Expresión permitida UPPER()
+        $query8 = DbQuery::table('series')
+            ->orderBy('UPPER(descripcion)');
+        $expected8 = 'SELECT * FROM ' . $this->db()->escapeColumn('series')
+            . ' ORDER BY UPPER(descripcion) ASC';
+        $this->assertEquals($expected8, $query8->sql());
+
+        // Test 9: Expresión permitida CAST()
+        $query9 = DbQuery::table('series')
+            ->orderBy('CAST(codserie AS INTEGER)');
+        $expected9 = 'SELECT * FROM ' . $this->db()->escapeColumn('series')
+            . ' ORDER BY CAST(codserie AS INTEGER) ASC';
+        $this->assertEquals($expected9, $query9->sql());
+
+        // Test 10: Múltiples orderBy
+        $query10 = DbQuery::table('series')
+            ->orderBy('codserie', 'ASC')
+            ->orderBy('lower:descripcion', 'DESC');
+        $expected10 = 'SELECT * FROM ' . $this->db()->escapeColumn('series')
+            . ' ORDER BY ' . $this->db()->escapeColumn('codserie') . ' ASC, '
+            . 'LOWER(' . $this->db()->escapeColumn('descripcion') . ') DESC';
+        $this->assertEquals($expected10, $query10->sql());
+    }
+
     public function testCount(): void
     {
         // si no existe la tabla países, saltamos el test