Skip to content

Commit

Permalink
Doc: prevent XSS via args (#7056)
Browse files Browse the repository at this point in the history
* Use third-person singular

It is my failure since #7029. Sorry...

* Doc: prevent XSS via args

An external name of method arg is not HTML-escaped (and missing quotes even
if needed). We can do XSS by this easily. Otherwise, `typeof` restriction is
not HTML-escaped, we can also do XSS by this.
  • Loading branch information
makenowjust authored and RX14 committed Nov 12, 2018
1 parent 52b2e6b commit c0a6716
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 10 deletions.
36 changes: 33 additions & 3 deletions spec/compiler/crystal/tools/doc/macro_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe Doc::Macro do
doc_macro.args_to_s.should eq("(**foo)")
end

it "show simple arg and double splat arg" do
it "shows simple arg and double splat arg" do
program = Program.new
generator = Doc::Generator.new program, ["."], ".", nil
doc_type = Doc::Type.new generator, program
Expand All @@ -62,7 +62,7 @@ describe Doc::Macro do
doc_macro.args_to_s.should eq("(foo, **bar)")
end

it "show block arg" do
it "shows block arg" do
program = Program.new
generator = Doc::Generator.new program, ["."], ".", nil
doc_type = Doc::Type.new generator, program
Expand All @@ -72,7 +72,7 @@ describe Doc::Macro do
doc_macro.args_to_s.should eq("(&foo)")
end

it "show simple arg and block arg" do
it "shows simple arg and block arg" do
program = Program.new
generator = Doc::Generator.new program, ["."], ".", nil
doc_type = Doc::Type.new generator, program
Expand All @@ -81,5 +81,35 @@ describe Doc::Macro do
doc_macro = Doc::Macro.new generator, doc_type, a_macro
doc_macro.args_to_s.should eq("(foo, &bar)")
end

it "shows external name of arg" do
program = Program.new
generator = Doc::Generator.new program, ["."], ".", nil
doc_type = Doc::Type.new generator, program

a_macro = Macro.new "foo", ["foo".arg(external_name: "bar")]
doc_macro = Doc::Macro.new generator, doc_type, a_macro
doc_macro.args_to_s.should eq("(bar foo)")
end

it "shows external name of arg with quotes and escaping" do
program = Program.new
generator = Doc::Generator.new program, ["."], ".", nil
doc_type = Doc::Type.new generator, program

a_macro = Macro.new "foo", ["foo".arg(external_name: "<<-< uouo fish life")]
doc_macro = Doc::Macro.new generator, doc_type, a_macro
doc_macro.args_to_s.should eq("(&quot;&lt;&lt;-&lt; uouo fish life&quot; foo)")
end

it "shows default value with highlighting" do
program = Program.new
generator = Doc::Generator.new program, ["."], ".", nil
doc_type = Doc::Type.new generator, program

a_macro = Macro.new "foo", ["foo".arg(default_value: 1.int32)]
doc_macro = Doc::Macro.new generator, doc_type, a_macro
doc_macro.args_to_s.should eq(%((foo = <span class="n">1</span>)))
end
end
end
40 changes: 40 additions & 0 deletions spec/compiler/crystal/tools/doc/method_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,45 @@ describe Doc::Method do
doc_method = Doc::Method.new generator, doc_type, a_def, false
doc_method.args_to_s.should eq("(foo) : Foo")
end

it "shows external name of arg" do
program = Program.new
generator = Doc::Generator.new program, ["."], ".", nil
doc_type = Doc::Type.new generator, program

a_def = Def.new "foo", ["foo".arg(external_name: "bar")]
doc_method = Doc::Method.new generator, doc_type, a_def, false
doc_method.args_to_s.should eq("(bar foo)")
end

it "shows external name of arg with quotes and escaping" do
program = Program.new
generator = Doc::Generator.new program, ["."], ".", nil
doc_type = Doc::Type.new generator, program

a_def = Def.new "foo", ["foo".arg(external_name: "<<-< uouo fish life")]
doc_method = Doc::Method.new generator, doc_type, a_def, false
doc_method.args_to_s.should eq("(&quot;&lt;&lt;-&lt; uouo fish life&quot; foo)")
end

it "shows typeof restriction of arg with highlighting" do
program = Program.new
generator = Doc::Generator.new program, ["."], ".", nil
doc_type = Doc::Type.new generator, program

a_def = Def.new "foo", ["foo".arg(restriction: TypeOf.new([1.int32] of ASTNode))]
doc_method = Doc::Method.new generator, doc_type, a_def, false
doc_method.args_to_s.should eq(%((foo : <span class="k">typeof</span>(<span class="n">1</span>))))
end

it "shows default value of arg with highlighting" do
program = Program.new
generator = Doc::Generator.new program, ["."], ".", nil
doc_type = Doc::Type.new generator, program

a_def = Def.new "foo", ["foo".arg(default_value: 1.int32)]
doc_method = Doc::Method.new generator, doc_type, a_def, false
doc_method.args_to_s.should eq(%((foo = <span class="n">1</span>)))
end
end
end
4 changes: 2 additions & 2 deletions spec/support/syntax.cr
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ class String
Var.new self
end

def arg(default_value = nil, restriction = nil)
Arg.new self, default_value: default_value, restriction: restriction
def arg(default_value = nil, restriction = nil, external_name = nil)
Arg.new self, default_value: default_value, restriction: restriction, external_name: external_name
end

def call
Expand Down
27 changes: 24 additions & 3 deletions src/compiler/crystal/tools/doc/macro.cr
Original file line number Diff line number Diff line change
Expand Up @@ -72,26 +72,47 @@ class Crystal::Doc::Macro
@macro.args.each_with_index do |arg, i|
io << ", " if printed
io << '*' if @macro.splat_index == i
io << arg
arg_to_s arg, io
printed = true
end

if double_splat = @macro.double_splat
io << ", " if printed
io << "**"
io << double_splat
arg_to_s double_splat, io
printed = true
end

if block_arg = @macro.block_arg
io << ", " if printed
io << '&'
io << block_arg
arg_to_s block_arg, io
end

io << ')'
end

def arg_to_s(arg : Arg, io)
if arg.external_name != arg.name
name = arg.external_name.empty? ? "_" : arg.external_name
if Symbol.needs_quotes? name
HTML.escape name.inspect, io
else
io << name
end
io << ' '
end

io << arg.name

# Macro arg cannot not have a restriction.

if default_value = arg.default_value
io << " = "
io << Highlighter.highlight(default_value.to_s)
end
end

def has_args?
!@macro.args.empty? || @macro.double_splat || @macro.block_arg
end
Expand Down
8 changes: 7 additions & 1 deletion src/compiler/crystal/tools/doc/method.cr
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,12 @@ class Crystal::Doc::Method

def arg_to_html(arg : Arg, io, links = true)
if arg.external_name != arg.name
io << (arg.external_name.empty? ? '_' : arg.external_name)
name = arg.external_name.empty? ? "_" : arg.external_name
if Symbol.needs_quotes? name
HTML.escape name.inspect, io
else
io << name
end
io << ' '
end

Expand All @@ -185,6 +190,7 @@ class Crystal::Doc::Method
io << " : "
@type.type_to_html type, io, links: links
end

if default_value = arg.default_value
io << " = "
io << Highlighter.highlight(default_value.to_s)
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/tools/doc/type.cr
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ class Crystal::Doc::Type
end

def node_to_html(node, io, links = true)
io << node
io << Highlighter.highlight(node.to_s)
end

def type_to_html(type)
Expand Down

0 comments on commit c0a6716

Please sign in to comment.