Infinite loop introduced in 1.5.4

[ghost]

Version 1.5.4 introduced an infinite loop in certain scenario's.

Line triggering the issue: v1.5.2...v1.5.4#diff-132c1f6c93734535818a6b91db4260dfd1c0433a9a5ce32e9012d9f05d9f7887R96

Reason:

1. Function is now calling Net_DNS2_Names::unpack and $offset should be updated by reference.
2. In Net_DNS2_Names::unpack, a 'return null' is given without updating $offset
3. Infinite loop

[evgenyneverov]

Same for me

[neroux]

Just tagging @mikepultz for visibility

[dee0912]

Same for me

[chaosben]

Same for me 🤓

Code to reproduce:

<?php
require 'vendor/autoload.php';
$resolver = new Net_DNS2_Resolver(['nameservers' => ['8.8.8.8']]);
$resolver->query('peopleinsight.co.uk', 'TXT');
//same here:
//$resolver->query('nl-sent.de', 'TXT');
//$resolver->query('algarvedreamproperty.com', 'TXT');
//$resolver->query('perpetual.com.au', 'TXT');
//$resolver->query('manage401k.com', 'TXT');

Fix in Names.php:

public static function unpack($rdata, &$offset)
    {
        if ($offset > strlen($rdata)) {
            return null;
        }

        $name = '';

        $len = ord($rdata[$offset++]);
        if ($len == 0) {
            return null;
        }

        //$offset++; 
        //moved above to $len = ord($rdata[$offset++]);

        if (($len + $offset) > strlen($rdata)) {

            $name = substr($rdata, $offset);
        } else {

            $name = substr($rdata, $offset, $len);
        }

        $offset += strlen($name);

        return $name;
    }

[ampenidas]

@mikepultz we are using this library in our project and having problems related to this ticket. can you tag the library with this fix? or should we fork?

[evgenyneverov]

@mikepultz we are using this library in our project and having problems related to this ticket. can you tag the library with this fix? or should we fork?

I've used pear/net_dns2^1.5.2, which has no issue and works just fine.

[Blackbam]

@mikepultz Also had to switch back to pear/net_dns2^1.5.2: Is it going to be fixed?

[dheineman]

We actually ran into the "...multi-line TXT records that might happen to have a trailing period before the line break" issue that exists in 1.5.2 but is fixed in 1.5.3/1.5.4 and as we need it fixed we decided to upgrade to 1.5.4 and added a custom patch based on chaosben's fix using vaimo/composer-patches.

# patches/fix_infinite_loop.patch

@package pear/net_dns2

diff --git a/Net/DNS2/Names.php b/Net/DNS2/Names.php
index 2cc7831..2dc26dc 100644
--- a/Net/DNS2/Names.php
+++ b/Net/DNS2/Names.php
@@ -83,13 +83,11 @@ class Net_DNS2_Names
 
         $name = '';
 
-        $len = ord($rdata[$offset]);
+        $len = ord($rdata[$offset++]);
         if ($len == 0)
         {
             return null;
         }
-        
-        $offset++;
 
         if ( ($len + $offset) > strlen($rdata)) {

It has been running in production just fine for the last 3 weeks now.

[Pascal76]

very annoying bug :(
I hope it will be officially fixed before the 1 year birthday ...
(Thank you for the fix guys, it works well)

[evgenyneverov]

Feel free to fix it =)

[neroux]

@evgenyneverov, I am not quite sure what the point of your comment is, as the issue is not how to fix it, but to fix it. @mikepultz has not responded to this issue so far and there actually is a fix by @dheineman.

This needs to added to the official code and this is not something @Pascal76 can do, so I am not sure what you were trying to say here.