Skip to content

Commit

Permalink
fixes a possible 'javascript:' protocol exploit [backport:1.0] (#19134)
Browse files Browse the repository at this point in the history
* 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>
(cherry picked from commit 9338aa2)
  • Loading branch information
Araq authored and narimiran committed Dec 10, 2021
1 parent 83c472c commit 4627512
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 5 deletions.
15 changes: 14 additions & 1 deletion lib/packages/docutils/rstgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -823,6 +823,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",
Expand Down Expand Up @@ -887,6 +897,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>
Expand Down Expand Up @@ -1187,6 +1199,7 @@ proc renderHyperlink(d: PDoc, text, link: PRstNode, result: var string, external
d.escMode = emUrl
renderRstToOut(d, link, linkStr)
d.escMode = mode
safeProtocol(linkStr)
var textStr = ""
renderRstToOut(d, text, textStr)
if external:
Expand Down
35 changes: 31 additions & 4 deletions tests/stdlib/trstgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ Some chapter
Level2
------
Level3
~~~~~~
Expand All @@ -407,7 +407,7 @@ Some chapter
More
~~~~
Another
-------
Expand Down Expand Up @@ -683,7 +683,7 @@ Test1
test "RST line blocks":
let input2 = dedent"""
Paragraph1
|
Paragraph2"""
Expand All @@ -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
Expand Down Expand Up @@ -1548,3 +1548,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>))""")

0 comments on commit 4627512

Please sign in to comment.