This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko.git
The following commit(s) were added to refs/heads/main by this push:
new 8391a2212e fix: bound compression-pointer hops in DNS name parsing
(#3490)
8391a2212e is described below
commit 8391a2212e5239b3719ac9d8c55179683375ec9c
Author: PJ Fanning <[email protected]>
AuthorDate: Mon Aug 31 11:03:00 2026 +0100
fix: bound compression-pointer hops in DNS name parsing (#3490)
Motivation:
DomainName.parse followed RFC 1035 compression pointers by recursing with
no hop limit and no check that the pointer moves backwards. A response whose
pointers form a cycle recursed until StackOverflowError, which is fatal and
escapes actor supervision. Message.parse runs on every datagram before the
transaction-id check, so any packet delivered to the resolver socket could
trigger it.
Modification:
Parse iteratively over a swappable iterator, cap the number of pointers
followed at 16 (a 255-octet name cannot legitimately need more), reject the
reserved 0x40/0x80 label types instead of reading them as negative lengths,
and reject names over 255 characters. Add DomainNameSpec.
Result:
Malformed or cyclic names fail with IllegalArgumentException, which actor
supervision handles, instead of overflowing the stack.
Tests:
- sbt "actor-tests/testOnly org.apache.pekko.io.dns.internal.DomainNameSpec
org.apache.pekko.io.dns.internal.MessageSpec
org.apache.pekko.io.dns.internal.DnsClientSpec" - 14 passed
- sbt actor/scalafmt actor-tests/Test/scalafmt,
actor-tests/Test/headerCreate
References:
None - found while reviewing the draft threat model in #3478
---
.../pekko/io/dns/internal/DomainNameSpec.scala | 67 ++++++++++++++++++++++
.../apache/pekko/io/dns/internal/DomainName.scala | 46 +++++++++++----
2 files changed, 101 insertions(+), 12 deletions(-)
diff --git
a/actor-tests/src/test/scala/org/apache/pekko/io/dns/internal/DomainNameSpec.scala
b/actor-tests/src/test/scala/org/apache/pekko/io/dns/internal/DomainNameSpec.scala
new file mode 100644
index 0000000000..d099e8fc13
--- /dev/null
+++
b/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)
+ }
+ }
+}
diff --git
a/actor/src/main/scala/org/apache/pekko/io/dns/internal/DomainName.scala
b/actor/src/main/scala/org/apache/pekko/io/dns/internal/DomainName.scala
index e0e92d7c0e..ec989ef450 100644
--- a/actor/src/main/scala/org/apache/pekko/io/dns/internal/DomainName.scala
+++ b/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")
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
