fix: bound compression-pointer hops in DNS name parsing (#3490)

actor-tests/src/test/scala/org/apache/pekko/io/dns/internal/DomainNameSpec.scala

@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.pekko.io.dns.internal
+
+import org.apache.pekko
+import pekko.util.ByteString
+
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+
+class DomainNameSpec extends AnyWordSpec with Matchers {
+
+  private def label(s: String): Seq[Byte] = s.length.toByte +: s.getBytes("US-ASCII").toSeq
+
+  private def pointer(offset: Int): Seq[Byte] =
+    Seq((0xC0 | (offset >> 8)).toByte, (offset & 0xFF).toByte)
+
+  "DomainName.parse" should {
+    "parse a plain name" in {
+      val bytes = ByteString((label("www") ++ label("example") ++ label("com") :+ 0.toByte): _*)
+      DomainName.parse(bytes.iterator, bytes) should be("www.example.com")
+    }
+
+    "follow a backwards compression pointer" in {
+      // offset 0: example.com   offset 13: www + pointer to offset 0
+      val bytes = ByteString((label("example") ++ label("com") :+ 0.toByte) ++ label("www") ++ pointer(0): _*)
+      DomainName.parse(bytes.iterator.drop(13), bytes) should be("www.example.com")
+    }
+
+    "reject a pointer that points at itself" in {
+      // offset 0: www, offset 4: pointer to offset 4
+      val bytes = ByteString(label("www") ++ pointer(4): _*)
+      an[IllegalArgumentException] should be thrownBy DomainName.parse(bytes.iterator, bytes)
+    }
+
+    "reject a pointer cycle" in {
+      // offset 0: a + pointer to offset 4, offset 4: b + pointer to offset 0
+      val bytes = ByteString(label("a") ++ pointer(4) ++ label("b") ++ pointer(0): _*)
+      an[IllegalArgumentException] should be thrownBy DomainName.parse(bytes.iterator, bytes)
+    }
+
+    "reject reserved label types" in {
+      val bytes = ByteString(0x40.toByte, 'a'.toByte, 0.toByte)
+      an[IllegalArgumentException] should be thrownBy DomainName.parse(bytes.iterator, bytes)
+    }
+
+    "fail rather than loop on a truncated name" in {
+      val bytes = ByteString(label("www"): _*)
+      a[NoSuchElementException] should be thrownBy DomainName.parse(bytes.iterator, bytes)
+    }
+  }
+}

actor/src/main/scala/org/apache/pekko/io/dns/internal/DomainName.scala

@@ -22,6 +22,16 @@ import pekko.util.{ ByteIterator, ByteString, ByteStringBuilder }
  */
 @InternalApi
 private[pekko] object DomainName {
+
+  /**
+   * RFC 1035 section 2.3.4 limits a name to 255 octets, so a well-formed name can never
+   * contain more than a handful of compression pointers. Bounding the number of pointers
+   * followed guarantees that `parse` terminates even for a response whose pointers form a
+   * cycle, which would otherwise recurse until the stack overflows.
+   */
+  private val MaxPointerHops = 16
+  private val MaxNameLength = 255
+
   def length(name: String): Short = {
     (name.length + 2).toShort
   }
@@ -38,24 +48,36 @@ private[pekko] object DomainName {
 
   def parse(it: ByteIterator, msg: ByteString): String = {
     val ret = new StringBuilder()
+    var current = it
+    var hops = 0
     while (true) {
-      val length = it.getByte
+      val length = current.getByte
       if (length == 0) {
-        val r = ret.result()
-        return r
+        return ret.result()
       }
 
-      if (ret.nonEmpty)
-        ret.append('.')
-
       if ((length & 0xC0) == 0xC0) {
-        val offset = ((length.toShort & 0x3F) << 8) | (it.getByte.toShort & 0x00FF)
-        return ret.result() + parse(msg.iterator.drop(offset), msg)
+        // compression pointer: the remainder of the name lives at `offset` in the message
+        hops += 1
+        if (hops > MaxPointerHops)
+          throw new IllegalArgumentException(
+            s"Unable to parse domain name: more than $MaxPointerHops compression pointers, probable pointer loop")
+        val offset = ((length & 0x3F) << 8) | (current.getByte & 0xFF)
+        current = msg.iterator.drop(offset)
+      } else if ((length & 0xC0) != 0) {
+        // 0x40 and 0x80 label types are reserved (RFC 1035 section 4.1.4)
+        throw new IllegalArgumentException(
+          s"Unable to parse domain name: unsupported label type [${(length & 0xC0) >> 6}]")
+      } else {
+        if (ret.nonEmpty)
+          ret.append('.')
+        ret.appendAll(current.clone().take(length).map(_.toChar))
+        current.drop(length)
+        if (ret.length > MaxNameLength)
+          throw new IllegalArgumentException(
+            s"Unable to parse domain name: name longer than $MaxNameLength characters")
       }
-
-      ret.appendAll(it.clone().take(length).map(_.toChar))
-      it.drop(length)
     }
-    throw new RuntimeException(s"Unable to parse domain name from msg: $msg")
+    throw new IllegalStateException(s"Unable to parse domain name from msg: $msg")
   }
 }