FacturaScripts: Stored XSS in WidgetVariante and WidgetSubcuenta modal lists via HTML-attribute decoding of Tools::noHtml-escaped quotes inside onclick=
Summary
WidgetVariante::renderVariantList (Core/Lib/Widget/WidgetVariante.php:298-330) and WidgetSubcuenta::renderSubaccountList (Core/Lib/Widget/WidgetSubcuenta.php:290-321) build the <tr onclick="..."> row for each modal hit by concatenating the user-controlled referencia / codsubcuenta field directly into a single-quoted JavaScript string literal inside an HTML onclick attribute. The defender's intuition is that Tools::noHtml (called in Variante::test() and Subcuenta::test()) replaces ' with the HTML entity ', neutralising the JavaScript string break. The intuition is wrong: HTML attribute parsing decodes character references before the JavaScript fragment is parsed, so ' becomes a literal ' in the JavaScript context. An attacker who can store a value such as 1',alert(1),'2 in Variante.referencia (no special characters required, just one apostrophe) ends up with widgetVarianteSelect('id', '1',alert(1),'2'); executing in any user's browser the moment they open the variant-picker modal.
The recent 40bc701 and 8586b97 fixes corrected the same anti-pattern in three sister classes by switching to data-reference="..." + this.dataset.reference. The two widget classes audited here were missed by that fix wave.
Details
the offending code
Core/Lib/Widget/WidgetVariante.php:298-330:
```php
protected function renderVariantList(): string
{
$items = [];
foreach ($this->variantes() as $item) {
$match = $item->{$this->match};
$description = Tools::textBreak($item->description(), 300);
...
$items[] = '<tr class="clickableRow" onclick="widgetVarianteSelect(\'' . $this->id . '\', \'' . $match . '\');">'
. '<td class="text-center">'
...
```
$this->match defaults to 'referencia' (WidgetVariante::__construct, line 42). $item->referencia was sanitised at write time by Variante::test() (Core/Model/Variante.php:392) which calls Tools::noHtml($this->referencia). Tools::noHtml (Core/Tools.php:499-504) replaces ', ", <, > with ', ", <, >. The defender therefore expects that any apostrophe a user typed becomes ' in the database, which renders inside the onclick attribute as ' and cannot break out of the surrounding '...' JS string literal.
Core/Lib/Widget/WidgetSubcuenta.php:290-305 has the identical shape:
```php
foreach ($this->subcuentas() as $item) {
$match = $item->{$this->match};
...
$items[] = '<tr class="clickableRow" onclick="widgetSubaccountSelect(\'' . $this->id . '\', \'' . $match . '\');">'
. '<td class="text-center">'
. '<a href="' . $item->url() . '" target="_blank" onclick="event.stopPropagation();">'
...
```
$this->match defaults to 'codsubcuenta'; the value is Tools::noHtml-encoded by Subcuenta::test() (Core/Model/Subcuenta.php:213).
why HTML-entity escaping does not protect a JavaScript string
Per the HTML5 spec (and what every browser actually does), the value of an HTML attribute is processed by the character reference state of the tokenizer before any consumer sees it. By the time the onclick attribute value reaches the script engine, the bytes inside are the *decoded* string. Concretely, the HTML the browser receives is:
```html
<tr onclick="widgetVarianteSelect('id', '1',alert(1),'2');">
```
After the tokenizer decodes ' to ', the JavaScript fragment passed to the script engine is:
```javascript
widgetVarianteSelect('id', '1',alert(1),'2');
```
alert(1) runs as a third positional argument that JavaScript happily evaluates while building the call. The widgetVarianteSelect function ends up being called with four arguments and the side-effect of alert(1) (or any payload) has already occurred.
The recent 40bc701 AccountingModalHTML and 8586b97 SalesModalHTML / PurchasesModalHTML fix recognised this. Both replaced the onclick="...('"+ value +"')" pattern with:
```php
$tbody .= '<tr ... data-subaccount="' . $code . '" onclick="$(...).modal(\'hide\');'
. ' return newLineAction(this.dataset.subaccount);">'
```
Where $code = static::html($subaccount->codsubcuenta) and static::html is htmlspecialchars(html_entity_decode($text, ENT_QUOTES | ENT_HTML5, 'UTF-8'), ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8'). The HTML5 entity decode is deliberate: it normalises any double-encoded data so that the subsequent htmlspecialchars produces stable single-encoded output. The JavaScript then reads the value from this.dataset.*, which is the post-decoded attribute value, where the original quote is now literally inside a string property and cannot break out of any quote context.
WidgetSubcuenta and WidgetVariante were not migrated to this pattern.
ways to plant t
