Fix XSS vulnerability within the JavaScript relocator

If the relocation placeholder appeared multiple times within the source code,
it would also be replaced multiple times. This might allow an attacker to blow
up the HTML structure by including the placeholder within UGC.

Fix this issue by only ever replacing the last placeholder, which should be the
“real” one from footer.tpl. In the future this should be protected further by
including a random nonce to prevent this attack entirely.

diff --git a/wcfsetup/install/files/lib/util/HeaderUtil.class.php b/wcfsetup/install/files/lib/util/HeaderUtil.class.php
index 05e1de49fa241ff8731bfba8fdd8ddda576227d1..771959b62d556036113c063f1630a27c5e79a2c6 100644 (file)
--- a/wcfsetup/install/files/lib/util/HeaderUtil.class.php
+++ b/wcfsetup/install/files/lib/util/HeaderUtil.class.php
@@ -132,7 +132,12 @@ final class HeaderUtil {
                        return '';
                }, self::$output);
                
-               self::$output = str_replace('<!-- JAVASCRIPT_RELOCATE_POSITION -->', implode("\n", $javascript), self::$output);
+               $placeholder = '<!-- JAVASCRIPT_RELOCATE_POSITION -->';
+               if (($placeholderPosition = \strrpos(self::$output, $placeholder)) !== false) {
+                       self::$output = \substr(self::$output, 0, $placeholderPosition)
+                               . \implode("\n", $javascript)
+                               . \substr(self::$output, $placeholderPosition + \strlen($placeholder));
+               }
                
                // 3rd party plugins may differ the actual output before it is sent to the browser
                // please be aware, that $eventObj is not available here due to this being a static