Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes a possible 'javascript:' protocol exploit [backport:1.0] #19134

Merged
merged 5 commits into from
Dec 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -825,6 +825,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 @@ -889,6 +899,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 @@ -1190,6 +1202,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: ""
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 @@ -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>))""")