From 48b0aea1d573d9e01752f9909ec8cd3a1b7e3378 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Wed, 2 Oct 2024 13:53:26 -0700
Subject: [PATCH] Issue #3457781 by catch, longwave, senscybersecurity, cmlara,
cilefen, poker10, greggles, alexpott, ericgsmith, xjm: Maintenance pages leak
sensitive environment information
---
core/includes/errors.inc | 42 +++++++++++--------
core/install.php | 5 +++
.../Functional/System/SystemAuthorizeTest.php | 17 ++++++++
3 files changed, 47 insertions(+), 17 deletions(-)
diff --git a/core/includes/errors.inc b/core/includes/errors.inc
index 2015dbd3e899..3ae65771f5b9 100644
--- a/core/includes/errors.inc
+++ b/core/includes/errors.inc
@@ -146,7 +146,7 @@ function error_displayable($error = NULL) {
* Non-recoverable fatal errors cannot be logged by Drupal.
*/
function _drupal_log_error($error, $fatal = FALSE) {
- $is_installer = InstallerKernel::installationAttempted();
+ $is_installer = InstallerKernel::installationAttempted() && \Drupal::hasContainer();
// Backtrace, exception and 'severity_level' are not valid replacement values
// for t().
@@ -157,7 +157,7 @@ function _drupal_log_error($error, $fatal = FALSE) {
// When running inside the testing framework, we relay the errors
// to the tested site by the way of HTTP headers.
- if (DRUPAL_TEST_IN_CHILD_SITE && !headers_sent() && (!defined('SIMPLETEST_COLLECT_ERRORS') || SIMPLETEST_COLLECT_ERRORS)) {
+ if (defined('DRUPAL_TEST_IN_CHILD_SITE') && DRUPAL_TEST_IN_CHILD_SITE && !headers_sent() && (!defined('SIMPLETEST_COLLECT_ERRORS') || SIMPLETEST_COLLECT_ERRORS)) {
_drupal_error_header($error['@message'], $error['%type'], $error['%function'], $error['%file'], $error['%line']);
}
@@ -173,7 +173,7 @@ function _drupal_log_error($error, $fatal = FALSE) {
// implementations to use it.
\Drupal::logger('php')->log($severity, '%type: @message in %function (line %line of %file) @backtrace_string.', $error + ['backtrace' => $backtrace, 'exception' => $exception, 'severity_level' => $severity]);
}
- catch (\Exception) {
+ catch (\Throwable) {
// We can't log, for example because the database connection is not
// available. At least try to log to PHP error log.
error_log(strtr('Failed to log error: ' . Error::DEFAULT_ERROR_MESSAGE . ' @backtrace_string', $error));
@@ -222,12 +222,16 @@ function _drupal_log_error($error, $fatal = FALSE) {
}
// Attempt to reduce verbosity by removing DRUPAL_ROOT from the file path
- // in the message. This does not happen for (false) security.
- if (\Drupal::hasService('kernel')) {
- $root_length = strlen(\Drupal::root());
- if (substr($error['%file'], 0, $root_length) == \Drupal::root()) {
- $error['%file'] = substr($error['%file'], $root_length + 1);
- }
+ // in the message. This also prevents full path disclosure, see
+ // https://owasp.org/www-community/attacks/Full_Path_Disclosure.
+ try {
+ $root = \Drupal::root();
+ }
+ catch (\Throwable) {
+ $root = realpath(dirname(__DIR__, 2));
+ }
+ if (str_starts_with($error['%file'], $root)) {
+ $error['%file'] = substr($error['%file'], strlen($root) + 1);
}
// Check if verbose error reporting is on.
@@ -243,14 +247,13 @@ function _drupal_log_error($error, $fatal = FALSE) {
}
else {
// With verbose logging, we will also include a backtrace.
-
// First trace is the error itself, already contained in the message.
- // While the second trace is the error source and also contained in the
- // message, the message doesn't contain argument values, so we output it
- // once more in the backtrace.
array_shift($backtrace);
- // Generate a backtrace containing only scalar argument values.
- $error['@backtrace'] = Error::formatBacktrace($backtrace);
+ // Strip arguments from the backtrace.
+ $error['@backtrace'] = Error::formatBacktrace(array_map(function ($trace) {
+ unset($trace['args']);
+ return $trace;
+ }, $backtrace));
$message = new FormattableMarkup('<details class="error-with-backtrace"><summary>' . Error::DEFAULT_ERROR_MESSAGE . '</summary><pre class="backtrace">@backtrace</pre></details>', $error);
}
}
@@ -267,8 +270,13 @@ function _drupal_log_error($error, $fatal = FALSE) {
'#title' => 'Error',
'#markup' => $message,
];
- install_display_output($output, $GLOBALS['install_state']);
- exit;
+ try {
+ install_display_output($output, $GLOBALS['install_state']);
+ exit;
+ }
+ catch (\Throwable) {
+ // The maintenance page failed, so fall back to a plain error message.
+ }
}
$response->setContent($message);
diff --git a/core/install.php b/core/install.php
index 550665d73f1f..e123db239734 100644
--- a/core/install.php
+++ b/core/install.php
@@ -43,6 +43,11 @@
exit();
}
+// Set the Drupal custom error handler.
+require_once $root_path . '/core/includes/errors.inc';
+set_error_handler('_drupal_error_handler');
+set_exception_handler('_drupal_exception_handler');
+
// Start the installer.
require_once $root_path . '/core/includes/install.core.inc';
install_drupal($class_loader);
diff --git a/core/modules/system/tests/src/Functional/System/SystemAuthorizeTest.php b/core/modules/system/tests/src/Functional/System/SystemAuthorizeTest.php
index fd8da499ebb4..d0d040cf054b 100644
--- a/core/modules/system/tests/src/Functional/System/SystemAuthorizeTest.php
+++ b/core/modules/system/tests/src/Functional/System/SystemAuthorizeTest.php
@@ -68,4 +68,21 @@ public function testFileTransferHooks(): void {
$this->assertSession()->responseContains('core/misc/states.js');
}
+ /**
+ * Tests error handling in authorize.php.
+ */
+ public function testError(): void {
+ $settings_filename = $this->siteDirectory . '/settings.php';
+ chmod($settings_filename, 0777);
+ $settings_php = file_get_contents($settings_filename);
+ $settings_php .= "\ndefine('SIMPLETEST_COLLECT_ERRORS', FALSE);\n";
+ $settings_php .= "\ntrigger_error('Test warning', E_USER_WARNING);\n";
+ file_put_contents($settings_filename, $settings_php);
+
+ $this->drupalGetAuthorizePHP();
+
+ $this->assertSession()->pageTextContains('User warning: Test warning');
+ $this->assertSession()->pageTextMatches('@line \d+ of sites/simpletest@');
+ }
+
}
--
GitLab