Skip to content

Commit

Permalink
[SECURITY] Avoid showing password hashes in backend edit forms
Browse files Browse the repository at this point in the history
Backend form fields of TCA `type=password` should never expose
the persisted value - especially, in case the value is explicitly
configured not to be hashed (having TCA `hashed=false`).

Resolves: #101965
Releases: main, 13.0, 12.4, 11.5
Change-Id: Ie05a708185c621b8a2120ad7851ac4caf180893f
Security-Bulletin: TYPO3-CORE-SA-2024-003
Security-References: CVE-2024-25118
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82941
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
  • Loading branch information
ohader committed Feb 13, 2024
1 parent 84e07e3 commit c7a135c
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 4 deletions.
21 changes: 19 additions & 2 deletions typo3/sysext/backend/Classes/Form/Element/InputTextElement.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public function render()
if ($config['readOnly'] ?? false) {
// Early return for read only fields
if (in_array('password', $evalList, true)) {
$itemValue = $itemValue ? '*********' : '';
$itemValue = $this->getObfuscatedSecretValue($itemValue);
}

$disabledFieldAttributes = [
Expand Down Expand Up @@ -240,6 +240,7 @@ public function render()
$fieldWizardHtml = $fieldWizardResult['html'];
$resultArray = $this->mergeChildReturnIntoExistingResult($resultArray, $fieldWizardResult, false);
$inputType = 'text';
$hiddenElementProps = ' value="' . htmlspecialchars((string)$itemValue) . '"';

if (in_array('email', $evalList, true)) {
$inputType = 'email';
Expand All @@ -255,14 +256,15 @@ public function render()
}
if (in_array('password', $evalList, true)) {
$attributes['spellcheck'] = 'false';
$hiddenElementProps = ' value="' . htmlspecialchars($this->getObfuscatedSecretValue($itemValue)) . '" disabled data-enable-on-modification="true"';
}

$mainFieldHtml = [];
$mainFieldHtml[] = '<div class="form-control-wrap" style="max-width: ' . $width . 'px">';
$mainFieldHtml[] = '<div class="form-wizards-wrap">';
$mainFieldHtml[] = '<div class="form-wizards-element">';
$mainFieldHtml[] = '<input type="' . $inputType . '" ' . GeneralUtility::implodeAttributes($attributes, true) . ' />';
$mainFieldHtml[] = '<input type="hidden" name="' . $parameterArray['itemFormElName'] . '" value="' . htmlspecialchars((string)$itemValue) . '" />';
$mainFieldHtml[] = '<input type="hidden" name="' . htmlspecialchars($parameterArray['itemFormElName']) . '" ' . $hiddenElementProps . ' />';
$mainFieldHtml[] = '</div>';
if (!empty($valuePickerHtml) || !empty($valueSliderHtml) || !empty($fieldControlHtml)) {
$mainFieldHtml[] = '<div class="form-wizards-items-aside form-wizards-items-aside--field-control">';
Expand Down Expand Up @@ -342,6 +344,21 @@ public function render()
return $resultArray;
}

/**
* Obfuscated a (hashed) password secret with a static string.
*
* @todo
* + server-side password obfuscation value is `*********` (9 chars)
* + client-side password obfuscation value is `********` (8 chars)
*/
protected function getObfuscatedSecretValue(?string $value): string
{
if ($value === null || $value === '') {
return '';
}
return '*********';
}

/**
* @return LanguageService
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,9 @@ define([
for (i = 0; i < evalList.length; i++) {
formattedValue = FormEngineValidation.formatValue(evalList[i], formattedValue, config);
}

if ($mainField.prop('disabled') && $mainField.data('enableOnModification')) {
$mainField.prop('disabled', false);
}
$mainField.val(newValue);
// After updating the value of the main field, dispatch a "change" event to inform e.g. the "RequestUpdate"
// component, which always listens to the main field instead of the "human readable field", about it.
Expand Down Expand Up @@ -570,6 +572,9 @@ define([
modified = true;
}
if (modified) {
if ($field.prop('disabled') && $field.data('enableOnModification')) {
$field.prop('disabled', false);
}
$field.val(newValue);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ protected function simpleInputFieldsDataProvider(): array
'inputValue' => 'Kasper',
'expectedValue' => '********',
'expectedInternalValue' => 'Kasper',
'expectedValueAfterSave' => 'Kasper',
// even if `password_2` is not hashed, it never should expose the value
'expectedValueAfterSave' => '*********',
'comment' => '',
],
[
Expand Down

0 comments on commit c7a135c

Please sign in to comment.