From 7e37bb083bc609d5eb3d5f572ced5842c6acfa14 Mon Sep 17 00:00:00 2001 From: TomTriple Date: Sun, 24 Sep 2023 15:46:00 +0200 Subject: [PATCH] add xss protection as default (#2421) * * escape output by default for xss protection * escaping '/' is not needed as of owasp-guide * format --- .../zio/http/internal/OutputEncoder.scala | 4 +-- .../main/scala/zio/http/template/Dom.scala | 13 ++++++-- .../scala/zio/http/template/DomSpec.scala | 30 +++++++++++++++++++ 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/zio-http/src/main/scala/zio/http/internal/OutputEncoder.scala b/zio-http/src/main/scala/zio/http/internal/OutputEncoder.scala index 52f8ad3944..572082a5c9 100644 --- a/zio-http/src/main/scala/zio/http/internal/OutputEncoder.scala +++ b/zio-http/src/main/scala/zio/http/internal/OutputEncoder.scala @@ -24,7 +24,6 @@ private[http] object OutputEncoder { private val `>` = ">" private val `"` = """ private val `'` = "'" - private val `/` = "/" /** * Encode HTML characters that can cause XSS, according to OWASP @@ -32,7 +31,7 @@ private[http] object OutputEncoder { * https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#output-encoding-rules-summary * * Specification: Convert & to &, Convert < to <, Convert > to >, - * Convert " to ", Convert ' to ', Convert / to / + * Convert " to ", Convert ' to ' * * Only use this function to encode characters inside HTML context: * output' => `>` case '"' => `"` case '\'' => `'` - case '/' => `/` case _ @char => char.toString } diff --git a/zio-http/src/main/scala/zio/http/template/Dom.scala b/zio-http/src/main/scala/zio/http/template/Dom.scala index e92faa197f..0ad20889ba 100644 --- a/zio-http/src/main/scala/zio/http/template/Dom.scala +++ b/zio-http/src/main/scala/zio/http/template/Dom.scala @@ -16,6 +16,8 @@ package zio.http.template +import zio.http.internal.OutputEncoder + /** * Light weight DOM implementation that can be rendered as a html string. * @@ -40,6 +42,7 @@ sealed trait Dom { self => val elements = children.collect { case self: Dom.Element => self case self: Dom.Text => self + case self: Dom.Raw => self } val noElements = elements.isEmpty @@ -61,9 +64,11 @@ sealed trait Dom { self => else s"<$name ${attributes.mkString(" ")}>$inner" - case Dom.Text(data) => data - case Dom.Attribute(name, value) => s"""$name="$value"""" + case Dom.Text(data) => OutputEncoder.encodeHtml(data.toString) + case Dom.Attribute(name, value) => + s"""$name="${OutputEncoder.encodeHtml(value.toString)}"""" case Dom.Empty => "" + case Dom.Raw(raw) => raw } } @@ -76,10 +81,14 @@ object Dom { def text(data: CharSequence): Dom = Dom.Text(data) + def raw(raw: CharSequence): Dom = Dom.Raw(raw) + private[zio] final case class Element(name: CharSequence, children: Seq[Dom]) extends Dom private[zio] final case class Text(data: CharSequence) extends Dom + private[zio] final case class Raw(raw: CharSequence) extends Dom + private[zio] final case class Attribute(name: CharSequence, value: CharSequence) extends Dom private[zio] object Empty extends Dom diff --git a/zio-http/src/test/scala/zio/http/template/DomSpec.scala b/zio-http/src/test/scala/zio/http/template/DomSpec.scala index e0395bd415..39e173480b 100644 --- a/zio-http/src/test/scala/zio/http/template/DomSpec.scala +++ b/zio-http/src/test/scala/zio/http/template/DomSpec.scala @@ -86,6 +86,36 @@ object DomSpec extends ZIOHttpSpec { assertTrue(dom.encode == """zio-http""") }, + test("xss protection for text nodes") { + val dom = Dom.element( + "a", + Dom.attr("href", "http://www.zio-http.com"), + Dom.text(""""""), + ) + assertTrue( + dom.encode == """<script type="text/javascript">alert("xss")</script>""", + ) + }, + test("xss protection for attributes") { + val dom = Dom.element( + "a", + Dom.attr("href", """"""), + Dom.text("my link"), + ) + assertTrue( + dom.encode == """my link""", + ) + }, + test("raw output") { + val dom = Dom.element( + "a", + Dom.attr("href", "http://www.zio-http.com"), + Dom.raw(""""""), + ) + assertTrue( + dom.encode == """""", + ) + }, suite("Self Closing")( test("void") { checkAll(voidTagGen) { name =>