CVE-2022-2016: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Cross-site Scripting (XSS) - Reflected in GitHub repository neorazorx/facturascripts prior to 2022.1.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2022-06-09T16:15Z
Repository: https://github.com/neorazorx/facturascripts
Language: PHP
Files changed: Core/Model/Balance.php, Test/Core/Model/BalanceTest.php, Test/Core/Model/IdentificadorFiscalTest.php
Commit date: 2022-06-06 11:56:23 +0200
Commit message: Escaped the html of the balance fields before any other tests. Also added the corresponding unit test.
------
Escapado el html de los campos del balance antes de cualquier otro test. Añadido también el correspondiente test unitario.

Diff: diff --git a/Core/Model/Balance.php b/Core/Model/Balance.php
index 820e53d9e0..d84578430b 100644
--- a/Core/Model/Balance.php
+++ b/Core/Model/Balance.php
@@ -123,24 +123,28 @@ class Balance extends Base\ModelClass
         return 'balances';
     }
 
     public function test(): bool
     {
-        if (1 !== preg_match('/^[A-Z0-9_\+\.\-]{1,15}$/i', $this->codbalance)) {
+        // escapamos el html
+        $this->codbalance = self::toolBox()::utils()::noHtml($this->codbalance);
+        $this->descripcion1 = self::toolBox()::utils()::noHtml($this->descripcion1);
+        $this->descripcion2 = self::toolBox()::utils()::noHtml($this->descripcion2);
+        $this->descripcion3 = self::toolBox()::utils()::noHtml($this->descripcion3);
+        $this->descripcion4 = self::toolBox()::utils()::noHtml($this->descripcion4);
+        $this->descripcion4ba = self::toolBox()::utils()::noHtml($this->descripcion4ba);
+        $this->naturaleza = self::toolBox()::utils()::noHtml($this->naturaleza);
+
+        // comprobamos que tenga un código válido
+        if (empty($this->codbalance) || 1 !== preg_match('/^[A-Z0-9_\+\.\-]{1,15}$/i', $this->codbalance)) {
             $this->toolBox()->i18nLog()->error(
                 'invalid-alphanumeric-code',
                 ['%value%' => $this->codbalance, '%column%' => 'codbalance', '%min%' => '1', '%max%' => '15']
             );
             return false;
         }
 
-        $utils = $this->toolBox()->utils();
-        $this->descripcion1 = $utils->noHtml($this->descripcion1);
-        $this->descripcion2 = $utils->noHtml($this->descripcion2);
-        $this->descripcion3 = $utils->noHtml($this->descripcion3);
-        $this->descripcion4 = $utils->noHtml($this->descripcion4);
-        $this->descripcion4ba = $utils->noHtml($this->descripcion4ba);
         return parent::test();
     }
 
     public function url(string $type = 'auto', string $list = 'ListReportAccounting?activetab=List'): string
     {
diff --git a/Test/Core/Model/BalanceTest.php b/Test/Core/Model/BalanceTest.php
new file mode 100644
index 0000000000..d9950aae46
--- /dev/null
+++ b/Test/Core/Model/BalanceTest.php
@@ -0,0 +1,73 @@
+<?php
+/**
+ * This file is part of FacturaScripts
+ * Copyright (C) 2022 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
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+namespace FacturaScripts\Test\Core\Model;
+
+use FacturaScripts\Core\Model\Balance;
+use PHPUnit\Framework\TestCase;
+
+final class BalanceTest extends TestCase
+{
+    public function testCreate()
+    {
+        // creamos un balance
+        $balance = new Balance();
+        $balance->codbalance = 'test';
+        $balance->descripcion1 = 'test';
+        $balance->naturaleza = 'A';
+        $this->assertTrue($balance->save(), 'cant-save-balance');
+
+        // eliminamos
+        $this->assertTrue($balance->delete(), 'cant-delete-balance');
+    }
+
+    public function testCantCreateEmpty()
+    {
+        $balance = new Balance();
+        $this->assertFalse($balance->save(), 'cant-save-balance');
+    }
+
+    public function testHtmlOnFields()
+    {
+        $balance = new Balance();
+        $balance->codbalance = '<test>';
+        $balance->descripcion1 = '<test>';
+        $balance->descripcion2 = '<test>';
+        $balance->descripcion3 = '<test>';
+        $balance->descripcion4 = '<test>';
+        $balance->descripcion4ba = '<test>';
+        $balance->naturaleza = '<test>';
+        $this->assertFalse($balance->save(), 'cant-save-balance-with-html');
+
+        // cambiamos el codigo a un codigo válido
+        $balance->codbalance = 'test';
+        $this->assertTrue($balance->save(), 'cant-save-balance-2');
+
+        // comprobamos que el html se ha escapado
+        $this->assertEquals('&lt;test&gt;', $balance->descripcion1);
+        $this->assertEquals('&lt;test&gt;', $balance->descripcion2);
+        $this->assertEquals('&lt;test&gt;', $balance->descripcion3);
+        $this->assertEquals('&lt;test&gt;', $balance->descripcion4);
+        $this->assertEquals('&lt;test&gt;', $balance->descripcion4ba);
+        $this->assertEquals('&lt;test&gt;', $balance->naturaleza);
+
+        // eliminamos
+        $this->assertTrue($balance->delete(), 'cant-delete-balance');
+    }
+}
diff --git a/Test/Core/Model/IdentificadorFiscalTest.php b/Test/Core/Model/IdentificadorFiscalTest.php
index d37203fcc6..f7cfff130e 100644
--- a/Test/Core/Model/IdentificadorFiscalTest.php
+++ b/Test/Core/Model/IdentificadorFiscalTest.php
@@ -21,11 +21,11 @@ namespace FacturaScripts\Test\Core\Model;
 
 use FacturaScripts\Core\Model\IdentificadorFiscal;
 use FacturaScripts\Test\Core\LogErrorsTrait;
 use PHPUnit\Framework\TestCase;
 
-class IdentificadorFiscalTest extends TestCase
+final class IdentificadorFiscalTest extends TestCase
 {
     use LogErrorsTrait;
 
     public function testCreate()
     {

Vulnerable code:         if (1 !== preg_match('/^[A-Z0-9_\+\.\-]{1,15}$/i', $this->codbalance)) {
        $utils = $this->toolBox()->utils();
        $this->descripcion1 = $utils->noHtml($this->descripcion1);
        $this->descripcion2 = $utils->noHtml($this->descripcion2);
        $this->descripcion3 = $utils->noHtml($this->descripcion3);
        $this->descripcion4 = $utils->noHtml($this->descripcion4);
        $this->descripcion4ba = $utils->noHtml($this->descripcion4ba);
class IdentificadorFiscalTest extends TestCase
Fixed code:         // escapamos el html
        $this->codbalance = self::toolBox()::utils()::noHtml($this->codbalance);
        $this->descripcion1 = self::toolBox()::utils()::noHtml($this->descripcion1);
        $this->descripcion2 = self::toolBox()::utils()::noHtml($this->descripcion2);
        $this->descripcion3 = self::toolBox()::utils()::noHtml($this->descripcion3);
        $this->descripcion4 = self::toolBox()::utils()::noHtml($this->descripcion4);
        $this->descripcion4ba = self::toolBox()::utils()::noHtml($this->descripcion4ba);
        $this->naturaleza = self::toolBox()::utils()::noHtml($this->naturaleza);

        // comprobamos que tenga un código válido
        if (empty($this->codbalance) || 1 !== preg_match('/^[A-Z0-9_\+\.\-]{1,15}$/i', $this->codbalance)) {
<?php
/**
 * This file is part of FacturaScripts
 * Copyright (C) 2022 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
 * published by the Free Software Foundation, either version 3 of the
 * License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 * GNU Lesser General Public License for more details.
 *
 * You should have received a copy of the GNU Lesser General Public License
 * along with this program. If not, see <http://www.gnu.org/licenses/>.
 */

namespace FacturaScripts\Test\Core\Model;

use FacturaScripts\Core\Model\Balance;
use PHPUnit\Framework\TestCase;

final class BalanceTest extends TestCase
{
    public function testCreate()
    {
        // creamos un balance
        $balance = new Balance();
        $balance->codbalance = 'test';
        $balance->descripcion1 = 'test';
        $balance->naturaleza = 'A';
        $this->assertTrue($balance->save(), 'cant-save-balance');

        // eliminamos
        $this->assertTrue($balance->delete(), 'cant-delete-balance');
    }

    public function testCantCreateEmpty()
    {
        $balance = new Balance();
        $this->assertFalse($balance->save(), 'cant-save-balance');
    }

    public function testHtmlOnFields()
    {
        $balance = new Balance();
        $balance->codbalance = '<test>';
        $balance->descripcion1 = '<test>';
        $balance->descripcion2 = '<test>';
        $balance->descripcion3 = '<test>';
        $balance->descripcion4 = '<test>';
        $balance->descripcion4ba = '<test>';
        $balance->naturaleza = '<test>';
        $this->assertFalse($balance->save(), 'cant-save-balance-with-html');

        // cambiamos el codigo a un codigo válido
        $balance->codbalance = 'test';
        $this->assertTrue($balance->save(), 'cant-save-balance-2');

        // comprobamos que el html se ha escapado
        $this->assertEquals('&lt;test&gt;', $balance->descripcion1);
        $this->assertEquals('&lt;test&gt;', $balance->descripcion2);
        $this->assertEquals('&lt;test&gt;', $balance->descripcion3);
        $this->assertEquals('&lt;test&gt;', $balance->descripcion4);
        $this->assertEquals('&lt;test&gt;', $balance->descripcion4ba);
        $this->assertEquals('&lt;test&gt;', $balance->naturaleza);

        // eliminamos
        $this->assertTrue($balance->delete(), 'cant-delete-balance');
    }
}
final class IdentificadorFiscalTest extends TestCase