From: Brian Wolff <bawolff+wn@gmail.com>
Date: Thu, 20 Sep 2018 04:31:21 +0000 (+0000)
Subject: Deprecate QuickTemplate::msgHtml & fix phan-taint-warning in includes/skins
X-Git-Tag: 1.34.0-rc.0~4024
X-Git-Url: http://git.cyclocoop.org/data/Fool?a=commitdiff_plain;h=f05bf4054089ac5394e2fd7bc60cc356446d20a2;p=lhc%2Fweb%2Fwiklou.git

Deprecate QuickTemplate::msgHtml & fix phan-taint-warning in includes/skins

QuickTemplate::msgHtml() (And the weird override that does the
same thing a different way - BaseTemplate::msgHtml()) are inherently
unsafe as they echo out a raw html message. This is strongly
discouraged in modern code. According to codeSearch tool, nothing
uses these methods, and there is a "@private" annotation on the
QuickTemplate::msgHtml() docblock. Thus hard deprecating it.

Change-Id: I4e9e157e922a36787adef4d0bf7608605c27f0c4
---

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index fb2829ba07..7a3cb1508e 100644
--- a/RELEASE-NOTES-1.32
+++ b/RELEASE-NOTES-1.32
@@ -456,6 +456,9 @@ because of Phabricator reports.
   $wgTidyConfig instead.
 * All Tidy configurations other than Remex have been hard deprecated;
   future parsers will not emit compatible output for these configurations.
+* QuickTemplate::msgHtml() and BaseTemplate::msgHtml() have been deprecated
+  as they promote bad practises. I18n messages should always be properly
+  escaped.
 
 === Other changes in 1.32 ===
 * (T198811) The following tables have had their UNIQUE indexes turned into
diff --git a/includes/skins/BaseTemplate.php b/includes/skins/BaseTemplate.php
index 5c4d812889..64145adb3a 100644
--- a/includes/skins/BaseTemplate.php
+++ b/includes/skins/BaseTemplate.php
@@ -43,7 +43,15 @@ abstract class BaseTemplate extends QuickTemplate {
 		echo $this->getMsg( $str )->escaped();
 	}
 
+	/**
+	 * @param string $str
+	 * @warning You should never use this method. I18n messages should be escaped
+	 * @deprecated 1.32 Use ->msg() or ->msgWiki() instead.
+	 * @suppress SecurityCheck-XSS
+	 * @return-taint exec_html
+	 */
 	function msgHtml( $str ) {
+		wfDeprecated( __METHOD__, '1.32' );
 		echo $this->getMsg( $str )->text();
 	}
 
diff --git a/includes/skins/QuickTemplate.php b/includes/skins/QuickTemplate.php
index 969ac510fc..e4a247640e 100644
--- a/includes/skins/QuickTemplate.php
+++ b/includes/skins/QuickTemplate.php
@@ -75,6 +75,7 @@ abstract class QuickTemplate {
 	 * @param string $name Key for the data
 	 * @param mixed|null $default Optional default (or null)
 	 * @return mixed The value of the data requested or the deafult
+	 * @return-taint onlysafefor_htmlnoent
 	 */
 	public function get( $name, $default = null ) {
 		return $this->data[$name] ?? $default;
@@ -101,6 +102,7 @@ abstract class QuickTemplate {
 	/**
 	 * @private
 	 * @param string $str
+	 * @suppress SecurityCheck-DoubleEscaped $this->data can be either
 	 */
 	function text( $str ) {
 		echo htmlspecialchars( $this->data[$str] );
@@ -109,6 +111,7 @@ abstract class QuickTemplate {
 	/**
 	 * @private
 	 * @param string $str
+	 * @suppress SecurityCheck-XSS phan-taint-check cannot tell if $str is pre-escaped
 	 */
 	function html( $str ) {
 		echo $this->data[$str];
@@ -125,8 +128,13 @@ abstract class QuickTemplate {
 	/**
 	 * @private
 	 * @param string $msgKey
+	 * @warning You should never use this method. I18n messages should be escaped
+	 * @deprecated 1.32 Use ->msg() or ->msgWiki() instead.
+	 * @suppress SecurityCheck-XSS
+	 * @return-taint exec_html
 	 */
 	function msgHtml( $msgKey ) {
+		wfDeprecated( __METHOD__, '1.32' );
 		echo wfMessage( $msgKey )->text();
 	}
 
diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php
index 2f5e0c8325..e426f7f8c5 100644
--- a/includes/skins/Skin.php
+++ b/includes/skins/Skin.php
@@ -1608,6 +1608,7 @@ abstract class Skin extends ContextSource {
 	/**
 	 * Create a section edit link.
 	 *
+	 * @suppress SecurityCheck-XSS $links has keys of different taint types
 	 * @param Title $nt The title being linked to (may not be the same as
 	 *   the current page, if the section is included from a template)
 	 * @param string $section The designation of the section being pointed to,