From b3840bbb3e67bb733c0f862d9b01c2d575591831 Mon Sep 17 00:00:00 2001
From: Aaron Wells <aaronw@catalyst.net.nz>
Date: Thu, 14 Apr 2016 19:52:42 +1200
Subject: [PATCH] Bug 1570221 Don't print parameter values to logs when in
 production mode

The best way to prevent sensitive data from being printed to the logs
is to avoid printing the value of *any* parameter. For instance, a
password parameter may have an unusual name, or it may be passed
through a general-purpose function like "strlen()".

Since parameter values are useful for debugging, we can still print
them when not in production mode (although with known password
params still scrubbed out).

Note this patch both scrubs likely password params, and hides their
scrubbed value. That's mostly because I'm lazy, but it also obscures
the password's actual length.

Change-Id: I4a1ab4c89a169c6b29a7b63384c2412cee761ab7
behatnotneeded: Can't test with behat
(cherry picked from commit 9a2972495d55c55633f1fa10522cd567933ecf6f)
---
 htdocs/init.php                |  3 +++
 htdocs/lib/config-defaults.php | 14 ++++++++++++++
 htdocs/lib/errors.php          | 30 +++++++++++++++++++++++++++---
 3 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/htdocs/init.php b/htdocs/init.php
index 2c071fa96a..bed6b77b64 100644
--- a/htdocs/init.php
+++ b/htdocs/init.php
@@ -311,6 +311,9 @@ if (!get_config('productionmode')) {
     $CFG->developermode       = DEVMODE_DEBUGJS | DEVMODE_DEBUGCSS | DEVMODE_UNPACKEDJS;
     $CFG->perftofoot          = true;
     $CFG->nocache             = true;
+    if ($CFG->log_backtrace_print_args === null) {
+        $CFG->log_backtrace_print_args = true;
+    }
 }
 
 if (get_config('installed')) {
diff --git a/htdocs/lib/config-defaults.php b/htdocs/lib/config-defaults.php
index d2815509c6..cfb4b66d88 100644
--- a/htdocs/lib/config-defaults.php
+++ b/htdocs/lib/config-defaults.php
@@ -110,6 +110,20 @@ $cfg->log_environ_targets = LOG_TARGET_SCREEN | LOG_TARGET_ERRORLOG;
  */
 $cfg->log_backtrace_levels = LOG_LEVEL_WARN | LOG_LEVEL_ENVIRON;
 
+
+/**
+ * @global boolean $cfg->log_backtrace_print_args Whether or not to print the values of function & method
+ * arguments when printing a backtrace. This can be useful for debugging, but it is a mild security risk,
+ * because function parameters may include sensitive data such as passwords and private keys. (Though
+ * arguments whose names suggest that they contain passwords, will still be blanked out even if this
+ * feature is enabled.)
+ *
+ * A NULL value here tells Mahara to hide argument values when $cfg->productionmode is enabled, and to
+ * show them otherwise. A TRUE or FALSE tells Mahara to always show/hide argument values in backtraces
+ * regardless of the value of $cfg->productionmode.
+ */
+$cfg->log_backtrace_print_args = null;
+
 /**
  * @global int $cfg->error_reporting What level of errors to print to the Mahara logs. Gets passed directly
  * to the PHP function "error_reporting()".
diff --git a/htdocs/lib/errors.php b/htdocs/lib/errors.php
index f54520838f..225f301b43 100644
--- a/htdocs/lib/errors.php
+++ b/htdocs/lib/errors.php
@@ -290,6 +290,7 @@ function log_message ($message, $loglevel, $escape, $backtrace, $file=null, $lin
  * @access private
  */
 function log_build_backtrace($backtrace) {
+    global $CFG;
     $calls = array();
 
     // Remove the call to log_message
@@ -308,6 +309,19 @@ function log_build_backtrace($backtrace) {
 
         $args = '';
         if ($bt['args']) {
+            // Determine whether or not to print the values of the function's
+            // arguments (which may contain sensitive data).
+            // Still always print the values of the "include" pseudofunctions,
+            // though, so the stacktrace will make sense.
+            $showvalues = ($CFG->log_backtrace_print_args === true || in_array(
+                    $bt['function'],
+                    array(
+                        'require',
+                        'include',
+                        'require_once',
+                        'include_once'
+                    )
+            ));
             foreach ($bt['args'] as $arg) {
                 if (!empty($args)) {
                     $args .= ', ';
@@ -315,11 +329,21 @@ function log_build_backtrace($backtrace) {
                 switch (gettype($arg)) {
                     case 'integer':
                     case 'double':
-                        $args .= $arg;
+                        if ($showvalues) {
+                            $args .= $arg;
+                        }
+                        else {
+                            $args .= (gettype($arg));
+                        }
                         break;
                     case 'string':
-                        $arg = substr($arg, 0, 50) . ((strlen($arg) > 50) ? '...' : '');
-                        $args .= '"' . $arg . '"';
+                        if ($showvalues) {
+                            $arg = substr($arg, 0, 50) . ((strlen($arg) > 50) ? '...' : '');
+                            $args .= '"' . $arg . '"';
+                        }
+                        else {
+                            $args .= 'string(size ' . strlen($arg) . ')';
+                        }
                         break;
                     case 'array':
                         $args .= 'array(size ' . count($arg) . ')';
-- 
GitLab