Merge pull request #116 from clue-labs/ttl

Fix interpretation of TTL as UINT32 with most significant bit unset
reactphp · Nov 5, 2018 · 6f2f761 · 6f2f761
2 parents 6638313 + 9952957
commit 6f2f761
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 2 deletions.
diff --git a/src/Model/Record.php b/src/Model/Record.php
@@ -20,7 +20,8 @@ class Record
     public $class;
 
     /**
-     * @var int maximum TTL in seconds (UINT16)
+     * @var int maximum TTL in seconds (UINT32, most significant bit always unset)
+     * @link https://tools.ietf.org/html/rfc2181#section-8
      */
     public $ttl;
 

diff --git a/src/Protocol/Parser.php b/src/Protocol/Parser.php
@@ -155,6 +155,11 @@ public function parseAnswer(Message $message)
         list($ttl) = array_values(unpack('N', substr($message->data, $consumed, 4)));
         $consumed += 4;
 
+        // TTL is a UINT32 that must not have most significant bit set for BC reasons
+        if ($ttl < 0 || $ttl >= 1 << 31) {
+            $ttl = 0;
+        }
+
         list($rdLength) = array_values(unpack('n', substr($message->data, $consumed, 2)));
         $consumed += 2;
 
@@ -220,7 +225,6 @@ public function parseAnswer(Message $message)
         $message->consumed = $consumed;
 
         $name = implode('.', $labels);
-        $ttl = $this->signedLongToUnsignedLong($ttl);
         $record = new Record($name, $type, $class, $ttl, $rdata);
 
         $message->answers[] = $record;
@@ -324,6 +328,10 @@ public function getCompressedLabelOffset($data, $consumed)
         return array($peek & $mask, $consumed + 2);
     }
 
+    /**
+     * @deprecated unused, exists for BC only
+     * @codeCoverageIgnore
+     */
     public function signedLongToUnsignedLong($i)
     {
         return $i & 0x80000000 ? $i - 0xffffffff : $i;

diff --git a/tests/Protocol/ParserTest.php b/tests/Protocol/ParserTest.php
@@ -157,6 +157,81 @@ public function testParseAnswerWithInlineData()
         $this->assertSame('178.79.169.131', $response->answers[0]->data);
     }
 
+    public function testParseAnswerWithExcessiveTtlReturnsZeroTtl()
+    {
+        $data = "";
+        $data .= "04 69 67 6f 72 02 69 6f 00";          // answer: igor.io
+        $data .= "00 01 00 01";                         // answer: type A, class IN
+        $data .= "ff ff ff ff";                         // answer: ttl 2^32 - 1
+        $data .= "00 04";                               // answer: rdlength 4
+        $data .= "b2 4f a9 83";                         // answer: rdata 178.79.169.131
+
+        $data = $this->convertTcpDumpToBinary($data);
+
+        $response = new Message();
+        $response->header->set('anCount', 1);
+        $response->data = $data;
+
+        $this->parser->parseAnswer($response);
+
+        $this->assertCount(1, $response->answers);
+        $this->assertSame('igor.io', $response->answers[0]->name);
+        $this->assertSame(Message::TYPE_A, $response->answers[0]->type);
+        $this->assertSame(Message::CLASS_IN, $response->answers[0]->class);
+        $this->assertSame(0, $response->answers[0]->ttl);
+        $this->assertSame('178.79.169.131', $response->answers[0]->data);
+    }
+
+    public function testParseAnswerWithTtlExactlyBoundaryReturnsZeroTtl()
+    {
+        $data = "";
+        $data .= "04 69 67 6f 72 02 69 6f 00";          // answer: igor.io
+        $data .= "00 01 00 01";                         // answer: type A, class IN
+        $data .= "80 00 00 00";                         // answer: ttl 2^31
+        $data .= "00 04";                               // answer: rdlength 4
+        $data .= "b2 4f a9 83";                         // answer: rdata 178.79.169.131
+
+        $data = $this->convertTcpDumpToBinary($data);
+
+        $response = new Message();
+        $response->header->set('anCount', 1);
+        $response->data = $data;
+
+        $this->parser->parseAnswer($response);
+
+        $this->assertCount(1, $response->answers);
+        $this->assertSame('igor.io', $response->answers[0]->name);
+        $this->assertSame(Message::TYPE_A, $response->answers[0]->type);
+        $this->assertSame(Message::CLASS_IN, $response->answers[0]->class);
+        $this->assertSame(0, $response->answers[0]->ttl);
+        $this->assertSame('178.79.169.131', $response->answers[0]->data);
+    }
+
+    public function testParseAnswerWithMaximumTtlReturnsExactTtl()
+    {
+        $data = "";
+        $data .= "04 69 67 6f 72 02 69 6f 00";          // answer: igor.io
+        $data .= "00 01 00 01";                         // answer: type A, class IN
+        $data .= "7f ff ff ff";                         // answer: ttl 2^31 - 1
+        $data .= "00 04";                               // answer: rdlength 4
+        $data .= "b2 4f a9 83";                         // answer: rdata 178.79.169.131
+
+        $data = $this->convertTcpDumpToBinary($data);
+
+        $response = new Message();
+        $response->header->set('anCount', 1);
+        $response->data = $data;
+
+        $this->parser->parseAnswer($response);
+
+        $this->assertCount(1, $response->answers);
+        $this->assertSame('igor.io', $response->answers[0]->name);
+        $this->assertSame(Message::TYPE_A, $response->answers[0]->type);
+        $this->assertSame(Message::CLASS_IN, $response->answers[0]->class);
+        $this->assertSame(0x7fffffff, $response->answers[0]->ttl);
+        $this->assertSame('178.79.169.131', $response->answers[0]->data);
+    }
+
     public function testParseAnswerWithUnknownType()
     {
         $data = "";