Skip to content

Commit

Permalink
Interpolated HTML attribute escaping improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelHatherly committed Jul 4, 2024
1 parent 2729753 commit 97e6185
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 10 deletions.
1 change: 1 addition & 0 deletions src/HypertextTemplates.jl
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const SRC_DIR = @__DIR__
# Includes.

include("Lexbor.jl")
include("SafeString.jl")
include("utilities.jl")
include("macro.jl")
include("nodes/abstract.jl")
Expand Down
19 changes: 19 additions & 0 deletions src/SafeString.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
"""
SafeString <: AbstractString
Represents an internal string type that was created directly within a template,
rather than being passed into a template from a potentially untrusted source as
a parameter. Never create these objects from application-level code.
"""
struct SafeString <: AbstractString
str::String
end

Base.show(io::IO, s::SafeString) = show(io, s.str)
Base.ncodeunits(s::SafeString) = ncodeunits(s.str)
Base.codeunit(s::SafeString) = codeunit(s.str)
Base.codeunit(s::SafeString, i::Integer) = codeunit(s.str, i)
Base.isvalid(s::SafeString, index::Integer) = isvalid(s.str, index)
Base.iterate(s::SafeString) = iterate(s.str)
Base.iterate(s::SafeString, state::Integer) = iterate(s.str, state)
Base.String(s::SafeString) = String(s.str)
13 changes: 11 additions & 2 deletions src/nodes/element.jl
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,18 @@ function expression(c::BuilderContext, a::Attribute)
elseif a.dynamic
return Expr(:kw, name, Meta.parse(a.value))
elseif a.interpolate
return Expr(:kw, name, Meta.parse("\"\"\"$(a.value)\"\"\""))
expr = Meta.parse("\"\"\"$(a.value)\"\"\"")
# When we interpolate prop values into another prop via Julia string
# interpolation with the `$prop=value` syntax, we want to perform HTML
# escaping on each value interpolated into the string, but not perform
# escaping on the hardcoded values that appear directly in the HTML
# template. This is what `_escape_html_str_expr` does. The resulting
# value is a `SafeString`, which will not be further HTML escaped it
# itself is interpolated into another string.
escaped_expr = _escape_html_attr_str_expr(expr)
return Expr(:kw, name, escaped_expr)
else
return Expr(:kw, name, a.value)
return Expr(:kw, name, SafeString(a.value))
end
end

Expand Down
39 changes: 33 additions & 6 deletions src/nodes/julia.jl
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,39 @@ function expression(c::BuilderContext, j::Julia)
end |> lln_replacer(c.file, j.line)
end

function escape_html(io::IO, value)
if showable("text/html", value)
show(io, "text/html", value)
else
escape_html(io, string(value))
end
function _escape_html_attr_str_expr(expr::Expr)
Meta.isexpr(expr, :string) || return expr
# When we encounter an interpolated string we wrap each interpolated
# value with a call to `_escape_html_attribute` which will escape the
# contents of the string. Strings that have already been escaped that
# are then interpolated into nested element attributes will be wrapped
# in a `SafeString` and will not be escaped again.
escaped_expr = Expr(:string, _escape_html_str_expr_part.(expr.args)...)
return Expr(:call, SafeString, escaped_expr)
end
_escape_html_attr_str_expr(other) = other

_escape_html_str_expr_part(string::String) = string
_escape_html_str_expr_part(other) = Expr(:call, _escape_html_attribute, other)

_escape_html_attribute(value) = sprint(escape_html, string(value))
# Strings marked as "safe" will not be escaped.
_escape_html_attribute(ss::SafeString) = ss

"""
escape_html(io::IO, value::T)
Provide a custom implementation of HTML escaping for a given type `T`. This is
useful if you have specific types within your application that you would like to
have handle their own HTML escaping rather than the default behavior where all
values are stringified and escaped.
You should only ever use this on types that you know are safe to be rendered
directly into HTML without escaping. Never pass user-generated content into
types that implement this method without first verifying and escaping content
manually.
"""
function escape_html end

function escape_html(io::IO, value::AbstractString)
for c in value
Expand All @@ -61,3 +87,4 @@ function escape_html(io::IO, value::AbstractString)
end
end
end
escape_html(io::IO, value) = escape_html(io, string(value))
24 changes: 24 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,30 @@ end
@test_reference joinpath(basic, "escaped-content.1.txt") render(
Templates.var"escaped-content";
content = "&='`<>/",
interpolated = "user-name",
)

@test_reference joinpath(basic, "escaped-content.2.txt") render(
Templates.var"escaped-content";
content = "&='`<>/",
interpolated = "\"></a><script>alert('xss')</script><a href=\"",
)

@test_reference joinpath(basic, "escaped-content.3.txt") render(
Templates.var"escaped-content";
content = cm"**bold**",
interpolated = "user-name",
)

struct HTMLObject
value::String
end
HypertextTemplates.escape_html(io::IO, obj::HTMLObject) = print(io, obj.value)

@test_reference joinpath(basic, "escaped-content.4.txt") render(
Templates.var"escaped-content";
content = HTMLObject(html(cm"**bold**")),
interpolated = "user-name",
)

@test_reference joinpath(basic, "custom-elements.1.txt") render(
Expand Down
2 changes: 1 addition & 1 deletion test/templates/basic/escaped-content.1.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<p>&amp;&#x3D;&#x27;&grave;&lt;&gt;&#x2F;</p>
<p>&amp;&#x3D;&#x27;&grave;&lt;&gt;&#x2F;</p><a href="/api/user-name">Link</a>
1 change: 1 addition & 0 deletions test/templates/basic/escaped-content.2.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>&amp;&#x3D;&#x27;&grave;&lt;&gt;&#x2F;</p><a href="/api/&quot;&gt;&lt;&#x2F;a&gt;&lt;script&gt;alert(&#x27;xss&#x27;)&lt;&#x2F;script&gt;&lt;a href&#x3D;&quot;">Link</a>
1 change: 1 addition & 0 deletions test/templates/basic/escaped-content.3.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>Node(CommonMark.Document)</p><a href="/api/user-name">Link</a>
2 changes: 2 additions & 0 deletions test/templates/basic/escaped-content.4.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<p><p><strong>bold</strong></p>
</p><a href="/api/user-name">Link</a>
3 changes: 2 additions & 1 deletion test/templates/basic/special-symbols.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
</div>
</function>

<function escaped-content content>
<function escaped-content content interpolated>
<p .julia=content></p>
<a $href="/api/$interpolated">Link</a>
</function>

0 comments on commit 97e6185

Please sign in to comment.