Do not allow entity references to refer to entities

core/util/src/main/scala/net/liftweb/util/SecurityHelpers.scala

@@ -17,12 +17,21 @@
 package net.liftweb
 package util
 
+import javax.xml.XMLConstants
+import javax.xml.parsers.SAXParserFactory
+import javax.xml.transform.stream.StreamSource
+import javax.xml.validation.SchemaFactory
+
 import org.apache.commons.codec.binary.Base64
-import java.io.{InputStream, ByteArrayOutputStream, ByteArrayInputStream, Reader, File, FileInputStream, BufferedReader}
-import java.security.{SecureRandom, MessageDigest}
+import java.io._
+import java.security._
 import javax.crypto._
 import javax.crypto.spec._
-import scala.xml.{Node, XML}
+import org.xml.sax.{EntityResolver, InputSource, ErrorHandler}
+import org.xml.sax.helpers.DefaultHandler
+
+import scala.xml.parsing.NoBindingFactoryAdapter
+import scala.xml.{TopScope, Elem, Node, XML}
 import common._
 
 object SecurityHelpers extends StringHelpers with IoHelpers with SecurityHelpers
@@ -75,23 +84,23 @@ trait SecurityHelpers {
   def md5(in: String): String = base64Encode(md5(in.getBytes("UTF-8")))
 
   /** create a SHA hash from a Byte array */
-  def hash(in : Array[Byte]) : Array[Byte] = {
+  def hash(in: Array[Byte]): Array[Byte] = {
     MessageDigest.getInstance("SHA").digest(in)
   }
 
   /** create a SHA hash from a String */
-  def hash(in: String) : String = {
+  def hash(in: String): String = {
     base64Encode(MessageDigest.getInstance("SHA").digest(in.getBytes("UTF-8")))
   }
 
-   /** create a SHA hash from a String */
-  def hashHex(in: String) : String = {
+  /** create a SHA hash from a String */
+  def hashHex(in: String): String = {
     Helpers.hexEncode(MessageDigest.getInstance("SHA").digest(in.getBytes("UTF-8")))
   }
 
   /** Compare two strings in a way that does not vary if the strings
-   * are determined to be not equal early (test every byte... avoids
-   * timing attackes */
+    * are determined to be not equal early (test every byte... avoids
+    * timing attackes */
   def secureEquals(s1: String, s2: String): Boolean = (s1, s2) match {
     case (null, null) => true
     case (null, _) => false
@@ -100,8 +109,8 @@ trait SecurityHelpers {
   }
 
   /** Compare two byte arrays in a way that does not vary if the arrays
-   * are determined to be not equal early (test every byte... avoids
-   * timing attackes */
+    * are determined to be not equal early (test every byte... avoids
+    * timing attackes */
   def secureEquals(s1: Array[Byte], s2: Array[Byte]): Boolean = (s1, s2) match {
     case (null, null) => true
     case (null, _) => false
@@ -121,12 +130,12 @@ trait SecurityHelpers {
 
 
   /** create a SHA-256 hash from a Byte array */
-  def hash256(in : Array[Byte]) : Array[Byte] = {
+  def hash256(in: Array[Byte]): Array[Byte] = {
     MessageDigest.getInstance("SHA-256").digest(in)
   }
 
   /** create a SHA-256 hash from a String */
-  def hash256(in : String): String = {
+  def hash256(in: String): String = {
     base64Encode(MessageDigest.getInstance("SHA-256").digest(in.getBytes("UTF-8")))
   }
 
@@ -164,7 +173,7 @@ trait SecurityHelpers {
       case 'd' | 'D' => 13
       case 'e' | 'E' => 14
       case 'f' | 'F' => 15
-        case _ => 0
+      case _ => 0
     }
 
     while (pos < max) {
@@ -197,5 +206,34 @@ trait SecurityHelpers {
     sb.toString
   }
 
+  def secureParseXML(in: String): Elem = secureParseXML(new StringReader(in))
+
+  def secureParseXML(in: Reader): Elem = {
+    val spf = SAXParserFactory.newInstance()
+    val saxParser = spf.newSAXParser();
+    val is = new InputSource(in);
+    val newAdapter = new SecureFactoryAdaptor
+    newAdapter.scopeStack.push( TopScope)
+
+    saxParser.parse(is, newAdapter)
+
+    newAdapter.scopeStack.pop
+    newAdapter.rootElem.asInstanceOf[Elem]
+  }
+
+  def secureParseXML(in: InputStream): Elem = secureParseXML(new InputStreamReader(in, "UTF-8"))
+
+
+  def tryIt = secureParseXML("""<?xml version="1.0" encoding="ISO-8859-1"?>
+ <!DOCTYPE foo [
+   <!ELEMENT foo ANY >
+   <!ENTITY xxe SYSTEM "file:///etc/passwd" >]><foo>&xxe;</foo>""")
+
 }
 
+private class SecureFactoryAdaptor extends NoBindingFactoryAdapter {
+  override def resolveEntity(publicId: String, systemId: String): InputSource = {
+    throw new SecurityException("Attempt to access an entity "+publicId+" / "+systemId)
+
+  }
+}

core/util/src/test/scala/net/liftweb/util/BindHelpersSpec.scala

@@ -326,7 +326,7 @@ object BindHelpersSpec extends Specification  {
     "handle dynamic, unprefixed attributes" in {
       // The Unprefixed attributes that Lift merges in cause the XML equals comparison to fail
       // stringifying and then reparsing fixes it.
-      XML.loadString(
+      Helpers.secureParseXML(
         BindHelpers.bind("test", 
                          <div><div test:x="dynamicUnprefixed" /></div>,
                          FuncAttrBindParam("x", {ns : NodeSeq => ns }, "id")).toString) must ==/(<div><div id="dynamicUnprefixed" /></div>)

core/util/src/test/scala/net/liftweb/util/SecurityHelpersSpec.scala

@@ -30,6 +30,27 @@ object SecurityHelpersSpec extends Specification  {
   "SecurityHelpers Specification".title
 
   "Security Helpers" should {
+    "Parse XML with nasty entity references" in {
+      (try {
+        Helpers.secureParseXML( """<?xml version="1.0" encoding="ISO-8859-1"?>
+ <!DOCTYPE foo [
+   <!ELEMENT foo ANY >
+   <!ENTITY xxe SYSTEM "file:///etc/passwd" >]><foo>&xxe;</foo>""")
+      } catch {
+        case e: SecurityException => "bad"
+      }) must_== "bad"
+    }
+
+      "Parse XML with deal with nice entity references" in {
+        (try { Helpers.secureParseXML("""<?xml version="1.0" encoding="ISO-8859-1"?>
+ <!DOCTYPE foo [
+   <!ELEMENT foo ANY >
+   <!ENTITY xxe SYSTEM "file:///etc/passwd" >]><foo>&amp;</foo>""")
+        } catch {
+          case e: SecurityException => "yes"
+        }).toString must_== "<foo>&amp;</foo>"
+    }
+
     "provide a randomLong method returning a random Long modulo a number" in {
       randomLong(7L) must be_<(7L)
     }

web/webkit/src/main/scala/net/liftweb/http/Req.scala

@@ -1021,7 +1021,7 @@ class Req(val path: ParsePath,
   else 
     try {
       import java.io._
-      body.map(b => XML.load(new ByteArrayInputStream(b)))
+      body.map(b => Helpers.secureParseXML(new ByteArrayInputStream(b)))
     } catch {
       case e: LiftFlowOfControlException => throw e
       case e: Exception => Failure(e.getMessage, Full(e), Empty)

web/webkit/src/test/scala/net/liftweb/builtin/snippet/MsgSpec.scala

@@ -17,6 +17,8 @@
 package net.liftweb
 package builtin.snippet
 
+import net.liftweb.util.Helpers
+
 import xml._
 import org.specs2.mutable.Specification
 
@@ -43,7 +45,7 @@ object MsgSpec extends Specification  {
 
         // We reparse due to inconsistencies with UnparsedAttributes
         val result = S.withAttrs(new UnprefixedAttribute("id", Text("foo"), new UnprefixedAttribute("noticeClass", Text("funky"), Null))) {
-          XML.loadString(Msg.render(<div/>).toString)
+          Helpers.secureParseXML(Msg.render(<div/>).toString)
         }
 
         result must ==/(<span id="foo">Error, <span class="funky">Notice</span></span>)

web/webkit/src/test/scala/net/liftweb/builtin/snippet/MsgsSpec.scala

@@ -17,6 +17,8 @@
 package net.liftweb
 package builtin.snippet
 
+import net.liftweb.util.Helpers
+
 import xml.XML
 import org.specs2.mutable.Specification
 
@@ -42,7 +44,7 @@ object MsgsSpec extends Specification  {
         S.notice("Notice")
 
         // We reparse due to inconsistencies with UnparsedAttributes
-        XML.loadString(Msgs.render(
+        Helpers.secureParseXML(Msgs.render(
           <lift:warning_msg>Warning:</lift:warning_msg><lift:notice_class>funky</lift:notice_class>
         ).toString)
       }