Reject parsing incomplete DNS response messages

Author: clue

src/Protocol/Parser.php

@@ -66,7 +66,7 @@ private function parse($data, Message $message)
 
     public function parseHeader(Message $message)
     {
-        if (strlen($message->data) < 12) {
+        if (!isset($message->data[12 - 1])) {
             return;
         }
 
@@ -97,19 +97,11 @@ public function parseHeader(Message $message)
 
     public function parseQuestion(Message $message)
     {
-        if (strlen($message->data) < 2) {
-            return;
-        }
-
         $consumed = $message->consumed;
 
         list($labels, $consumed) = $this->readLabels($message->data, $consumed);
 
-        if (null === $labels) {
-            return;
-        }
-
-        if (strlen($message->data) - $consumed < 4) {
+        if ($labels === null || !isset($message->data[$consumed + 4 - 1])) {
             return;
         }
 
@@ -133,19 +125,11 @@ public function parseQuestion(Message $message)
 
     public function parseAnswer(Message $message)
     {
-        if (strlen($message->data) < 2) {
-            return;
-        }
-
         $consumed = $message->consumed;
 
         list($labels, $consumed) = $this->readLabels($message->data, $consumed);
 
-        if (null === $labels) {
-            return;
-        }
-
-        if (strlen($message->data) - $consumed < 10) {
+        if ($labels === null || !isset($message->data[$consumed + 10 - 1])) {
             return;
         }
 
@@ -163,6 +147,10 @@ public function parseAnswer(Message $message)
         list($rdLength) = array_values(unpack('n', substr($message->data, $consumed, 2)));
         $consumed += 2;
 
+        if (!isset($message->data[$consumed + $rdLength - 1])) {
+            return;
+        }
+
         $rdata = null;
 
         if (Message::TYPE_A === $type || Message::TYPE_AAAA === $type) {
@@ -262,6 +250,7 @@ private function readLabels($data, $consumed)
 
                 $consumed += 2;
                 list($newLabels) = $this->readLabels($data, $offset);
+
                 if ($newLabels === null) {
                     return array(null, null);
                 }

tests/Protocol/ParserTest.php

@@ -690,6 +690,41 @@ public function testParseInvalidOffsetPointerToStartOfMessageInQuestionNameThrow
         $this->parser->parseMessage($data);
     }
 
+    /**
+     * @expectedException InvalidArgumentException
+     */
+    public function testParseIncompleteAnswerFieldsThrows()
+    {
+        $data = "";
+        $data .= "72 62 81 80 00 01 00 01 00 00 00 00"; // header
+        $data .= "04 69 67 6f 72 02 69 6f 00";          // question: igor.io
+        $data .= "00 01 00 01";                         // question: type A, class IN
+        $data .= "c0 0c";                               // answer: offset pointer to igor.io
+
+        $data = $this->convertTcpDumpToBinary($data);
+
+        $this->parser->parseMessage($data);
+    }
+
+    /**
+     * @expectedException InvalidArgumentException
+     */
+    public function testParseIncompleteAnswerRecordDataThrows()
+    {
+        $data = "";
+        $data .= "72 62 81 80 00 01 00 01 00 00 00 00"; // header
+        $data .= "04 69 67 6f 72 02 69 6f 00";          // question: igor.io
+        $data .= "00 01 00 01";                         // question: type A, class IN
+        $data .= "c0 0c";                               // answer: offset pointer to igor.io
+        $data .= "00 01 00 01";                         // answer: type A, class IN
+        $data .= "00 01 51 80";                         // answer: ttl 86400
+        $data .= "00 04";                               // answer: rdlength 4
+
+        $data = $this->convertTcpDumpToBinary($data);
+
+        $this->parser->parseMessage($data);
+    }
+
     private function convertTcpDumpToBinary($input)
     {
         // sudo ngrep -d en1 -x port 53