Prevent HTML and JS injection in section names

Before this change, HTML and some JS code (as far as it was not escaped by json_encode) could be injected into the output as the closing pattern that is checked by the regex is not escaped in JSON (see test case).
dokuwiki · Apr 15, 2018 · ada0d77 · ada0d77
1 parent 75c3272
commit ada0d77
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 2 deletions.
diff --git a/_test/tests/inc/html_secedit_pattern.test.php b/_test/tests/inc/html_secedit_pattern.test.php
@@ -56,4 +56,15 @@ public function testSecEditPattern($text, $expectedMatches, $msg) {
         }
     }
 
+    public function testSecEditHTMLInjection() {
+        $ins = p_get_instructions("====== Foo ======\n\n===== } --> <script> =====\n\n===== Bar =====\n");
+        $info = array();
+        $xhtml = p_render('xhtml', $ins, $info);
+
+        $this->assertNotNull($xhtml);
+
+        $xhtml_without_secedit = html_secedit($xhtml, false);
+
+        $this->assertFalse(strpos($xhtml_without_secedit, '<script>'), 'Plain <script> tag found in output - HTML/JS injection might be possible!');
+    }
 }
diff --git a/inc/html.php b/inc/html.php
@@ -118,7 +118,8 @@ function html_secedit($text,$show=true){
  * @triggers HTML_SECEDIT_BUTTON
  */
 function html_secedit_button($matches){
-    $data = json_decode($matches[1], true);
+    $json = htmlspecialchars_decode($matches[1], ENT_QUOTES);
+    $data = json_decode($json, true);
     if ($data == NULL) {
         return;
     }

diff --git a/inc/parser/xhtml.php b/inc/parser/xhtml.php
@@ -103,7 +103,7 @@ public function finishSectionEdit($end = null, $hid = null) {
         }
         $data['range'] = $data['start'].'-'.(is_null($end) ? '' : $end);
         unset($data['start']);
-        $this->doc .= '<!-- EDIT'.json_encode ($data).' -->';
+        $this->doc .= '<!-- EDIT'.hsc(json_encode ($data)).' -->';
     }
 
     /**