Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion core/csvbulkexport.class.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* @license http://opensource.org/licenses/AGPL-3.0
*/

use Combodo\iTop\Application\Helper\ExportHelper;
use Combodo\iTop\Application\UI\Base\Component\FieldSet\FieldSetUIBlockFactory;
use Combodo\iTop\Application\UI\Base\Component\Html\Html;
use Combodo\iTop\Application\UI\Base\Component\Input\InputUIBlockFactory;
Expand All @@ -13,7 +14,6 @@
use Combodo\iTop\Application\UI\Base\Component\Panel\PanelUIBlockFactory;
use Combodo\iTop\Application\UI\Base\Layout\MultiColumn\Column\ColumnUIBlockFactory;
use Combodo\iTop\Application\UI\Base\Layout\MultiColumn\MultiColumnUIBlockFactory;
use Combodo\iTop\Application\Helper\ExportHelper;
use Combodo\iTop\Application\WebPage\Page;
use Combodo\iTop\Application\WebPage\WebPage;

Expand Down Expand Up @@ -55,6 +55,8 @@ public function ReadParameters()
$this->aStatusInfo['charset'] = strtoupper(utils::ReadParam('charset', 'UTF-8', true, 'raw_data'));
$this->aStatusInfo['formatted_text'] = (bool)utils::ReadParam('formatted_text', 0, true);

$this->aStatusInfo['ignore_excel_sanitization'] = (bool)utils::ReadParam('ignore_excel_sanitization', 0, true, utils::ENUM_SANITIZATION_FILTER_INTEGER);

$sDateFormatRadio = utils::ReadParam('csv_date_format_radio', '');
switch ($sDateFormatRadio) {
case 'default':
Expand Down Expand Up @@ -223,6 +225,10 @@ public function GetFormPart(WebPage $oP, $sPartId)
$oRadioCustom->GetInput()->AddCSSClass('ibo-input-checkbox');
$oFieldSetDate->AddSubBlock($oRadioCustom);

$oFieldSetSecurity = FieldSetUIBlockFactory::MakeStandard(Dict::S('Core:BulkExport:Security'));
$oMulticolumn->AddColumn(ColumnUIBlockFactory::MakeForBlock($oFieldSetSecurity));
$oFieldSetSecurity->AddSubBlock(ExportHelper::GetInputForSanitizeExcelExport());

$oP->add_ready_script(
<<<EOF
$('#form_part_csv_options').on('preview_updated', function() { FormatDatesInPreview('csv', 'csv'); });
Expand Down Expand Up @@ -264,6 +270,13 @@ protected function GetValue($oObj, $sAttCode)
default:
$sRet = trim($oObj->GetAsCSV($sAttCode), '"');
}

// If the option to ignore Excel sanitization is not set or explicitly set to false, apply sanitization
if (!(array_key_exists('ignore_excel_sanitization', $this->aStatusInfo)) || $this->aStatusInfo['ignore_excel_sanitization'] === false) {
return ExportHelper::SanitizeField($sRet, $this->aStatusInfo['text_qualifier'] ?? '');
}

// The option to ignore Excel sanitization is explicitly set to true: return the raw value without sanitization
return $sRet;
}

Expand Down Expand Up @@ -337,6 +350,12 @@ public function GetNextChunk(&$aStatus)
$sField = $oObj->GetAsCSV($sAttCode, $this->aStatusInfo['separator'], $this->aStatusInfo['text_qualifier'], $this->bLocalizeOutput, !$this->aStatusInfo['formatted_text']);
}
}

// If the option to ignore Excel sanitization is not set or absent, sanitize the field
if (!(array_key_exists('ignore_excel_sanitization', $this->aStatusInfo)) || $this->aStatusInfo['ignore_excel_sanitization'] === false) {
$sField = ExportHelper::SanitizeField($sField, $this->aStatusInfo['text_qualifier']);
}

Comment on lines +354 to +358
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double sanitization issue: The field is being sanitized twice in GetNextChunk. Line 349 calls GetAsCSV which returns a value wrapped in text qualifiers (e.g., "=org1"). Then line 355 calls ExportHelper::SanitizeField with the text_qualifier parameter, which will add another single quote inside the qualifier, resulting in "'=org1" when it should just be "'=org1" without the outer qualifiers at this point. The sanitization should happen before the text qualifiers are added, not after. Consider removing this sanitization call since it's already handled in GetValue() on line 279, or ensure GetAsCSV is called with an empty text qualifier before sanitization.

Suggested change
// If the option to ignore Excel sanitization is not set or absent, sanitize the field
if (!(array_key_exists('ignore_excel_sanitization', $this->aStatusInfo)) || $this->aStatusInfo['ignore_excel_sanitization'] === false) {
$sField = ExportHelper::SanitizeField($sField, $this->aStatusInfo['text_qualifier']);
}

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetValue is not called in the GetNextChunk context in CSV export (it is though in the Excel export, that's why the code differs).

The sanitization does not add the text_qualifier, it strips them from the start of the value to compare its first character and then add it back.

I see no risk of double sanitization

if ($this->aStatusInfo['charset'] != 'UTF-8') {
// Note: due to bugs in the glibc library it's safer to call iconv on the smallest possible string
// and thus to convert field by field and not the whole row or file at once (see ticket N°991)
Expand Down
14 changes: 13 additions & 1 deletion core/excelbulkexport.class.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
* @license http://opensource.org/licenses/AGPL-3.0
*/

use Combodo\iTop\Application\Helper\ExportHelper;
use Combodo\iTop\Application\UI\Base\Component\FieldSet\FieldSetUIBlockFactory;
use Combodo\iTop\Application\UI\Base\Component\Html\Html;
use Combodo\iTop\Application\UI\Base\Component\Input\InputUIBlockFactory;
use Combodo\iTop\Application\UI\Base\Component\Panel\PanelUIBlockFactory;
use Combodo\iTop\Application\UI\Base\Layout\MultiColumn\Column\ColumnUIBlockFactory;
use Combodo\iTop\Application\UI\Base\Layout\MultiColumn\MultiColumnUIBlockFactory;
use Combodo\iTop\Application\Helper\ExportHelper;
use Combodo\iTop\Application\WebPage\Page;
use Combodo\iTop\Application\WebPage\WebPage;

Expand Down Expand Up @@ -63,6 +63,8 @@ public function ReadParameters()
// Export from the command line (or scripted) => default format is SQL, as in previous versions of iTop, unless specified otherwise
$this->aStatusInfo['date_format'] = utils::ReadParam('date_format', (string)AttributeDateTime::GetSQLFormat(), true, 'raw_data');
}

$this->aStatusInfo['ignore_excel_sanitization'] = (bool)utils::ReadParam('ignore_excel_sanitization', 0, true, utils::ENUM_SANITIZATION_FILTER_INTEGER);
}

public function EnumFormParts()
Expand Down Expand Up @@ -121,6 +123,10 @@ public function GetFormPart(WebPage $oP, $sPartId)
$oRadioCustom->GetInput()->AddCSSClass('ibo-input-checkbox');
$oFieldSetDate->AddSubBlock($oRadioCustom);

$oFieldSetSecurity = FieldSetUIBlockFactory::MakeStandard(Dict::S('Core:BulkExport:Security'));
$oMulticolumn->AddColumn(ColumnUIBlockFactory::MakeForBlock($oFieldSetSecurity));
$oFieldSetSecurity->AddSubBlock(ExportHelper::GetInputForSanitizeExcelExport());

$oP->add_ready_script(
<<<EOF
$('#form_part_xlsx_options').on('preview_updated', function() { FormatDatesInPreview('excel', 'xlsx'); });
Expand Down Expand Up @@ -216,6 +222,12 @@ protected function GetValue($oObj, $sAttCode)
}
}
}

// If the option to ignore Excel sanitization is not set or absent, sanitize the field
if (!(array_key_exists('ignore_excel_sanitization', $this->aStatusInfo)) || $this->aStatusInfo['ignore_excel_sanitization'] === false) {
return ExportHelper::SanitizeField($sRet, '');
}

return $sRet;
}

Expand Down
1 change: 1 addition & 0 deletions css/backoffice/application/bulk/_all.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
*/

@import "bulk-modify";
@import "bulk-export";
10 changes: 10 additions & 0 deletions css/backoffice/application/bulk/_bulk-export.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* @copyright Copyright (C) 2010-2026 Combodo SAS
* @license http://opensource.org/licenses/AGPL-3.0
*/

#form_part_csv_options:has(#ibo-sanitize-excel-export--input:checked), #form_part_xlsx_options:has(#ibo-sanitize-excel-export--input:checked){
#ibo-sanitize-excel-export--alert {
display: none;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</div>

<div id="export-feedback">
<p id="export-excel-warning" class="alert alert-warning" role="alert">{{ 'UI:Bulk:Export:MaliciousInjection:Alert:Message'|dict_format(sWikiUrl)|raw }}</p>
<p id="export-excel-warning" class="alert alert-warning" role="alert">{{ 'UI:Bulk:Export:MaliciousInjection:Sanitization:Alert:Message'|dict_format(sWikiUrl)|raw }}</p>
<p class="export-message" style="text-align:center;">{{ 'ExcelExport:PreparingExport'|dict_s }}</p>
<div class="progress">
<div class="progress-bar" role="progressbar" style="width: 0%"
Expand Down
6 changes: 5 additions & 1 deletion dictionaries/ui/application/bulk/cs.dictionary.itop.bulk.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,9 @@
Dict::Add('CS CZ', 'Czech', 'Čeština', [
'UI:Bulk:modify:IncompatibleAttribute' => 'This attribute can\'t be edited in bulk context~~',
'UI:Bulk:Export:MaliciousInjection:Alert:Title' => 'Excel security warning~~',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Opening a file with untrusted data in Microsoft Excel may lead to formula injection. Ensure that your Excel settings are configured to handle files safely. <a href="%1$s">Learn more in our documentation.</a>~~',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Opening a file with untrusted data in Microsoft Excel may lead to formula injection. Ensure that your Excel settings are configured to handle files safely. <a href="%1$s" target="_blank">Learn more in our documentation.</a>~~',
'UI:Bulk:Export:MaliciousInjection:Sanitization:Alert:Message' => 'Some values have been sanitized to prevent potential security issues in Microsoft Excel. <a href="%1$s" target="_blank">Learn more in our documentation.</a>~~',
'UI:Bulk:Export:MaliciousInjection:Input:Label' => 'Sanitize potentially dangerous values~~',
'UI:Bulk:Export:MaliciousInjection:Input:Tooltip' => 'When enabled, potentially dangerous values will be sanitized during export. This will prevent Microsoft Excel from interpreting them as formulas. Note that this may alter the original data by prefixing it with a single quote (\') to ensure it is treated as text.~~',
'Core:BulkExport:Security' => 'Security~~',
]);
6 changes: 5 additions & 1 deletion dictionaries/ui/application/bulk/da.dictionary.itop.bulk.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,9 @@
Dict::Add('DA DA', 'Danish', 'Dansk', [
'UI:Bulk:modify:IncompatibleAttribute' => 'This attribute can\'t be edited in bulk context~~',
'UI:Bulk:Export:MaliciousInjection:Alert:Title' => 'Excel security warning~~',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Opening a file with untrusted data in Microsoft Excel may lead to formula injection. Ensure that your Excel settings are configured to handle files safely. <a href="%1$s">Learn more in our documentation.</a>~~',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Opening a file with untrusted data in Microsoft Excel may lead to formula injection. Ensure that your Excel settings are configured to handle files safely. <a href="%1$s" target="_blank">Learn more in our documentation.</a>~~',
'UI:Bulk:Export:MaliciousInjection:Sanitization:Alert:Message' => 'Some values have been sanitized to prevent potential security issues in Microsoft Excel. <a href="%1$s" target="_blank">Learn more in our documentation.</a>~~',
'UI:Bulk:Export:MaliciousInjection:Input:Label' => 'Sanitize potentially dangerous values~~',
'UI:Bulk:Export:MaliciousInjection:Input:Tooltip' => 'When enabled, potentially dangerous values will be sanitized during export. This will prevent Microsoft Excel from interpreting them as formulas. Note that this may alter the original data by prefixing it with a single quote (\') to ensure it is treated as text.~~',
'Core:BulkExport:Security' => 'Security~~',
]);
6 changes: 5 additions & 1 deletion dictionaries/ui/application/bulk/de.dictionary.itop.bulk.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,9 @@
Dict::Add('DE DE', 'German', 'Deutsch', [
'UI:Bulk:modify:IncompatibleAttribute' => 'Dieses Attribut kann in einer Massenänderung nicht bearbeitet werden.',
'UI:Bulk:Export:MaliciousInjection:Alert:Title' => 'Excel-Sicherheitswarnung',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Das Öffnen einer Datei mit nicht vertrauenswürdigen Daten in Microsoft Excel kann zu einer Formel-Injektion führen. Stellen Sie sicher, dass Ihre Excel-Einstellungen so konfiguriert sind, dass Dateien sicher verarbeitet werden. <a href="%1$s">Erfahren Sie mehr in unserer Dokumentation.</a>',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Das Öffnen einer Datei mit nicht vertrauenswürdigen Daten in Microsoft Excel kann zu einer Formel-Injektion führen. Stellen Sie sicher, dass Ihre Excel-Einstellungen so konfiguriert sind, dass Dateien sicher verarbeitet werden. <a href="%1$s" target="_blank">Erfahren Sie mehr in unserer Dokumentation.</a>',
'UI:Bulk:Export:MaliciousInjection:Sanitization:Alert:Message' => 'Some values have been sanitized to prevent potential security issues in Microsoft Excel. <a href="%1$s" target="_blank">Learn more in our documentation.</a>~~',
'UI:Bulk:Export:MaliciousInjection:Input:Label' => 'Sanitize potentially dangerous values~~',
'UI:Bulk:Export:MaliciousInjection:Input:Tooltip' => 'When enabled, potentially dangerous values will be sanitized during export. This will prevent Microsoft Excel from interpreting them as formulas. Note that this may alter the original data by prefixing it with a single quote (\') to ensure it is treated as text.~~',
'Core:BulkExport:Security' => 'Security~~',
]);
6 changes: 5 additions & 1 deletion dictionaries/ui/application/bulk/en.dictionary.itop.bulk.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,9 @@
// Bulk modify
'UI:Bulk:modify:IncompatibleAttribute' => 'This attribute can\'t be edited in bulk context',
'UI:Bulk:Export:MaliciousInjection:Alert:Title' => 'Excel security warning',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Opening a file with untrusted data in Microsoft Excel may lead to formula injection. Ensure that your Excel settings are configured to handle files safely. <a href="%1$s">Learn more in our documentation.</a>',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Opening a file with untrusted data in Microsoft Excel may lead to formula injection. Ensure that your Excel settings are configured to handle files safely. <a href="%1$s" target="_blank">Learn more in our documentation.</a>',
'UI:Bulk:Export:MaliciousInjection:Sanitization:Alert:Message' => 'Some values have been sanitized to prevent potential security issues in Microsoft Excel. <a href="%1$s" target="_blank">Learn more in our documentation.</a>',
'UI:Bulk:Export:MaliciousInjection:Input:Label' => 'Sanitize potentially dangerous values',
'UI:Bulk:Export:MaliciousInjection:Input:Tooltip' => 'When enabled, potentially dangerous values will be sanitized during export. This will prevent Microsoft Excel from interpreting them as formulas. Note that this may alter the original data by prefixing it with a single quote (\') to ensure it is treated as text.',
'Core:BulkExport:Security' => 'Security',
]);
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,9 @@
// Bulk modify
'UI:Bulk:modify:IncompatibleAttribute' => 'This attribute can\'t be edited in bulk context',
'UI:Bulk:Export:MaliciousInjection:Alert:Title' => 'Excel security warning',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Opening a file with untrusted data in Microsoft Excel may lead to formula injection. Ensure that your Excel settings are configured to handle files safely. <a href="%1$s">Learn more in our documentation.</a>',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Opening a file with untrusted data in Microsoft Excel may lead to formula injection. Ensure that your Excel settings are configured to handle files safely. <a href="%1$s" target="_blank">Learn more in our documentation.</a>',
'UI:Bulk:Export:MaliciousInjection:Sanitization:Alert:Message' => 'Some values have been sanitized to prevent potential security issues in Microsoft Excel. <a href="%1$s" target="_blank">Learn more in our documentation.</a>',
'UI:Bulk:Export:MaliciousInjection:Input:Label' => 'Sanitize potentially dangerous values',
'UI:Bulk:Export:MaliciousInjection:Input:Tooltip' => 'When enabled, potentially dangerous values will be sanitized during export. This will prevent Microsoft Excel from interpreting them as formulas. Note that this may alter the original data by prefixing it with a single quote (\') to ensure it is treated as text.',
'Core:BulkExport:Security' => 'Security',
]);
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,9 @@
Dict::Add('ES CR', 'Spanish', 'Español, Castellano', [
'UI:Bulk:modify:IncompatibleAttribute' => 'Este atributo no se puede editar en contexto masivo',
'UI:Bulk:Export:MaliciousInjection:Alert:Title' => 'Advertencia de seguridad de Excel',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Abrir un archivo con datos que no son de confianza en Microsoft Excel puede provocar la inyección de fórmulas. Asegúrese de que la configuración de Excel esté configurada para manejar archivos de forma segura. <a href="%1$s">Obtenga más información en nuestra documentación.</a>',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Abrir un archivo con datos que no son de confianza en Microsoft Excel puede provocar la inyección de fórmulas. Asegúrese de que la configuración de Excel esté configurada para manejar archivos de forma segura. <a href="%1$s" target="_blank">Obtenga más información en nuestra documentación.</a>',
'UI:Bulk:Export:MaliciousInjection:Sanitization:Alert:Message' => 'Some values have been sanitized to prevent potential security issues in Microsoft Excel. <a href="%1$s" target="_blank">Learn more in our documentation.</a>~~',
'UI:Bulk:Export:MaliciousInjection:Input:Label' => 'Sanitize potentially dangerous values~~',
'UI:Bulk:Export:MaliciousInjection:Input:Tooltip' => 'When enabled, potentially dangerous values will be sanitized during export. This will prevent Microsoft Excel from interpreting them as formulas. Note that this may alter the original data by prefixing it with a single quote (\') to ensure it is treated as text.~~',
'Core:BulkExport:Security' => 'Security~~',
]);
6 changes: 5 additions & 1 deletion dictionaries/ui/application/bulk/fr.dictionary.itop.bulk.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,9 @@
Dict::Add('FR FR', 'French', 'Français', [
'UI:Bulk:modify:IncompatibleAttribute' => 'Cet attribut ne peut être édité dans une modification en masse',
'UI:Bulk:Export:MaliciousInjection:Alert:Title' => 'Avertissement sur la sécurité d\'Excel',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'L\'ouverture d\'un fichier contenant des données non fiables dans Microsoft Excel peut entraîner l\'injection de formules. Assurez-vous que vos paramètres Excel sont configurés pour traiter les fichiers en toute sécurité. <a href="%1$s">Pour en savoir plus, consultez notre documentation.</a>',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'L\'ouverture d\'un fichier contenant des données non fiables dans Microsoft Excel peut entraîner l\'injection de formules. Assurez-vous que vos paramètres Excel sont configurés pour traiter les fichiers en toute sécurité. <a href="%1$s" target="_blank">Pour en savoir plus, consultez notre documentation.</a>',
'UI:Bulk:Export:MaliciousInjection:Sanitization:Alert:Message' => 'Certaines valeurs ont été échappées pour prévenir de potentielles failles de sécurités dans Microsoft Excel. <a href="%1$s" target="_blank">Pour en savoir plus, consultez notre documentation</a>',
'UI:Bulk:Export:MaliciousInjection:Input:Label' => 'Sanitiser les valeurs potentiellement dangereuses',
'UI:Bulk:Export:MaliciousInjection:Input:Tooltip' => 'Lorsqu\'elle est activée, les valeurs potentiellement dangereuses seront sanitizées lors de l\'exportation. Cela empêchera Microsoft Excel de les interpréter comme des formules. Notez que cela peut altérer les données originales en les préfixant avec une simple quote (\') pour s\'assurer qu\'elles soient traitées comme du texte.',
'Core:BulkExport:Security' => 'Sécurité',
]);
6 changes: 5 additions & 1 deletion dictionaries/ui/application/bulk/hu.dictionary.itop.bulk.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,9 @@
Dict::Add('HU HU', 'Hungarian', 'Magyar', [
'UI:Bulk:modify:IncompatibleAttribute' => 'This attribute can\'t be edited in bulk context~~',
'UI:Bulk:Export:MaliciousInjection:Alert:Title' => 'Excel security warning~~',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Opening a file with untrusted data in Microsoft Excel may lead to formula injection. Ensure that your Excel settings are configured to handle files safely. <a href="%1$s">Learn more in our documentation.</a>~~',
'UI:Bulk:Export:MaliciousInjection:Alert:Message' => 'Opening a file with untrusted data in Microsoft Excel may lead to formula injection. Ensure that your Excel settings are configured to handle files safely. <a href="%1$s" target="_blank">Learn more in our documentation.</a>~~',
'UI:Bulk:Export:MaliciousInjection:Sanitization:Alert:Message' => 'Some values have been sanitized to prevent potential security issues in Microsoft Excel. <a href="%1$s" target="_blank">Learn more in our documentation.</a>~~',
'UI:Bulk:Export:MaliciousInjection:Input:Label' => 'Sanitize potentially dangerous values~~',
'UI:Bulk:Export:MaliciousInjection:Input:Tooltip' => 'When enabled, potentially dangerous values will be sanitized during export. This will prevent Microsoft Excel from interpreting them as formulas. Note that this may alter the original data by prefixing it with a single quote (\') to ensure it is treated as text.~~',
'Core:BulkExport:Security' => 'Security~~',
]);
Loading