summary refs log tree commit diff stats
diff options
context:
space:
mode:
authorAndreas Rumpf <rumpf_a@web.de>2021-12-10 09:24:20 +0100
committerGitHub <noreply@github.com>2021-12-10 09:24:20 +0100
commit9338aa24977e84a33b9a7802eaff0777fcf4d9c3 (patch)
tree367e510cba2bf50dcbb5dc4855d038954f5353ee
parentc98954233981cb30807bd8be8b805663d44964d1 (diff)
downloadNim-9338aa24977e84a33b9a7802eaff0777fcf4d9c3.tar.gz
fixes a possible 'javascript:' protocol exploit [backport:1.0] (#19134)
* fixes a possible 'javascript:' protocol exploit [backport:1.0]

* add tests

* Update tests/stdlib/trstgen.nim

* add the same logic for hyperlinks

* move the logic into a proc

Co-authored-by: narimiran <narimiran@disroot.org>
-rw-r--r--lib/packages/docutils/rstgen.nim15
-rw-r--r--tests/stdlib/trstgen.nim35
2 files changed, 45 insertions, 5 deletions
diff --git a/lib/packages/docutils/rstgen.nim b/lib/packages/docutils/rstgen.nim
index dc6ca25c1..9018087f7 100644
--- a/lib/packages/docutils/rstgen.nim
+++ b/lib/packages/docutils/rstgen.nim
@@ -40,7 +40,7 @@
 ##   can be done by simply searching for [footnoteName].
 
 import strutils, os, hashes, strtabs, rstast, rst, highlite, tables, sequtils,
-  algorithm, parseutils, std/strbasics
+  algorithm, parseutils, std/strbasics, strscans
 
 import ../../std/private/since
 
@@ -827,6 +827,16 @@ proc renderOverline(d: PDoc, n: PRstNode, result: var string) =
         rstnodeToRefname(n).idS, tmp, $chr(n.level - 1 + ord('A')), tocName])
 
 
+proc safeProtocol(linkStr: var string) =
+  var protocol = ""
+  if scanf(linkStr, "$w:", protocol):
+    # if it has a protocol at all, ensure that it's not 'javascript:' or worse:
+    if cmpIgnoreCase(protocol, "http") == 0 or cmpIgnoreCase(protocol, "https") == 0 or
+        cmpIgnoreCase(protocol, "ftp") == 0:
+      discard "it's fine"
+    else:
+      linkStr = ""
+
 proc renderTocEntry(d: PDoc, e: TocEntry, result: var string) =
   dispA(d.target, result,
     "<li><a class=\"reference\" id=\"$1_toc\" href=\"#$1\">$2</a></li>\n",
@@ -891,6 +901,8 @@ proc renderImage(d: PDoc, n: PRstNode, result: var string) =
 
   # support for `:target:` links for images:
   var target = esc(d.target, getFieldValue(n, "target").strip(), escMode=emUrl)
+  safeProtocol(target)
+
   if target.len > 0:
     # `htmlOut` needs to be of the following format for link to work for images:
     # <a class="reference external" href="target"><img src=\"$1\"$2/></a>
@@ -1192,6 +1204,7 @@ proc renderHyperlink(d: PDoc, text, link: PRstNode, result: var string,
     d.escMode = emUrl
     renderRstToOut(d, link, linkStr)
     d.escMode = mode
+  safeProtocol(linkStr)
   var textStr = ""
   renderRstToOut(d, text, textStr)
   let nimDocStr = if nimdoc: " nimdoc" else: ""
diff --git a/tests/stdlib/trstgen.nim b/tests/stdlib/trstgen.nim
index 45cf1a68e..2be65b35b 100644
--- a/tests/stdlib/trstgen.nim
+++ b/tests/stdlib/trstgen.nim
@@ -398,7 +398,7 @@ Some chapter
 
       Level2
       ------
-      
+
       Level3
       ~~~~~~
 
@@ -407,7 +407,7 @@ Some chapter
 
       More
       ~~~~
-      
+
       Another
       -------
 
@@ -683,7 +683,7 @@ Test1
   test "RST line blocks":
     let input2 = dedent"""
       Paragraph1
-      
+
       |
 
       Paragraph2"""
@@ -704,7 +704,7 @@ Test1
     # check that '|   ' with a few spaces is still parsed as new line
     let input4 = dedent"""
       | xxx
-      |      
+      |
       |     zzz"""
 
     let output4 = input4.toHtml
@@ -1549,3 +1549,30 @@ suite "RST/Code highlight":
 
     check strip(rstToHtml(pythonCode, {}, newStringTable(modeCaseSensitive))) ==
       strip(expected)
+
+
+suite "invalid targets":
+  test "invalid image target":
+    let input1 = dedent """.. image:: /images/myimage.jpg
+      :target: https://bar.com
+      :alt: Alt text for the image"""
+    let output1 = input1.toHtml
+    check output1 == """<a class="reference external" href="https://bar.com"><img src="/images/myimage.jpg" alt="Alt text for the image"/></a>"""
+
+    let input2 = dedent """.. image:: /images/myimage.jpg
+      :target: javascript://bar.com
+      :alt: Alt text for the image"""
+    let output2 = input2.toHtml
+    check output2 == """<img src="/images/myimage.jpg" alt="Alt text for the image"/>"""
+
+    let input3 = dedent """.. image:: /images/myimage.jpg
+      :target: bar.com
+      :alt: Alt text for the image"""
+    let output3 = input3.toHtml
+    check output3 == """<a class="reference external" href="bar.com"><img src="/images/myimage.jpg" alt="Alt text for the image"/></a>"""
+
+  test "invalid links":
+    check("(([Nim](https://nim-lang.org/)))".toHtml ==
+        """((<a class="reference external" href="https://nim-lang.org/">Nim</a>))""")
+    check("(([Nim](javascript://nim-lang.org/)))".toHtml ==
+        """((<a class="reference external" href="">Nim</a>))""")