hept_lookup() enters infinite loop on TCPTransport

[lgrangeia]

In certain situations, when performing a DCE RPC EPM lookup over the TCP Transport, the server will close the connection and the client will be stuck in an endless loop in the recv() method:

impacket/dcerpc/v5/transport.py

    def recv(self, forceRecv = 0, count = 0):
        if count:
            buffer = b''
            while len(buffer) < count:
               #stuck in loop here
               buffer += self.__socket.recv(count-len(buffer))
        else:
            buffer = self.__socket.recv(8192)
        return buffer

I created a very simple patch that fixes this issue but I'm not sure if it introduces others, please test.

diff --git a/impacket/dcerpc/v5/rpcrt.py b/impacket/dcerpc/v5/rpcrt.py
index dce7a523..f2e8fc50 100644
--- a/impacket/dcerpc/v5/rpcrt.py
+++ b/impacket/dcerpc/v5/rpcrt.py
@@ -1306,6 +1306,8 @@ class DCERPC_v5(DCERPC):
             # At least give me the MSRPCRespHeader, especially important for 
             # TCP/UDP Transports
             response_data = self._transport.recv(forceRecv, count=MSRPCRespHeader._SIZE)
+            if len(response_data) < MSRPCRespHeader._SIZE:
+                raise DCERPCException('No MSRPC Header received')
             response_header = MSRPCRespHeader(response_data)
             # Ok, there might be situation, especially with large packets, that 
             # the transport layer didn't send us the full packet's contents
diff --git a/impacket/dcerpc/v5/transport.py b/impacket/dcerpc/v5/transport.py
index 36c11c13..493ac713 100644
--- a/impacket/dcerpc/v5/transport.py
+++ b/impacket/dcerpc/v5/transport.py
@@ -373,7 +373,9 @@ class TCPTransport(DCERPCTransport):
         if count:
             buffer = b''
             while len(buffer) < count:
-               buffer += self.__socket.recv(count-len(buffer))
+                buffer += self.__socket.recv(count-len(buffer))
+                if len(buffer) == 0:
+                    return buffer
         else:
             buffer = self.__socket.recv(8192)
         return buffer

[mohemiv]

I'm a bit familiar with this code, so I would like to write my notes about it.

1.

--- a/impacket/dcerpc/v5/rpcrt.py
+++ b/impacket/dcerpc/v5/rpcrt.py
@@ -1306,6 +1306,8 @@ class DCERPC_v5(DCERPC):
             # At least give me the MSRPCRespHeader, especially important for 
             # TCP/UDP Transports
             response_data = self._transport.recv(forceRecv, count=MSRPCRespHeader._SIZE)
+            if len(response_data) < MSRPCRespHeader._SIZE:
+                raise DCERPCException('No MSRPC Header received')
             response_header = MSRPCRespHeader(response_data)
             # Ok, there might be situation, especially with large packets, that 
             # the transport layer didn't send us the full packet's contents

Here we have a problem that the code uses the MSRPCRespHeader structure, but actually the MSRPCHeader structure needs to be used. This does work somehow, but I believe there are issue which appear in some borderline cases.

The MSRPCHeader structure is smaller than MSRPCRespHeader, and it covers all the MSRPC packets, unlike MSRPCRespHeader.

For example, the server can send the shutdown PDU, which is exactly MSRPCHeader._SIZE bytes long ([C706] 12.6.4.11 The shutdown PDU).

2.

--- a/impacket/dcerpc/v5/transport.py
+++ b/impacket/dcerpc/v5/transport.py
@@ -373,7 +373,9 @@ class TCPTransport(DCERPCTransport):
         if count:
             buffer = b''
             while len(buffer) < count:
-               buffer += self.__socket.recv(count-len(buffer))
+                buffer += self.__socket.recv(count-len(buffer))
+                if len(buffer) == 0:
+                    return buffer
         else:
             buffer = self.__socket.recv(8192)
         return buffer

There is an issue that if you receive 1 byte, there will still be an infinite loop.

Also, I've checked the code, and I see almost no implementation and no usage of forceRecv and count arguments of the recv method. It would be interesting to know why these arguments were coded (@asolino), and if there is no reason for it, I think we should consider removing them and making the recv method implementations Unix-like.

[lgrangeia]

There is an issue that if you receive 1 byte, there will still be an infinite loop.

You are correct, this only covers the edge case of recv() receiving exactly zero bytes. I already encountered other situations where the infinite loop still happens even with this patch.

Let me know if you have worked on any other solution and please post any patches / workarounds here and I will be happy to test them.

[CravateRouge]

I think this issue should not be overlooked, I had the same problem testing the zerologon CVE using the Secura script zerologon_tester.py. The server seems to shutdown the connection as described here and nothing is done in the impacket recv() function to handle this corner case. I wrote the following and it works in my case but I don't know if it answers all cases:

    def recv(self, forceRecv = 0, count = 0):
        if count:
            buffer = b''
            bufferSize = 0
            while bufferSize < count:
               buffer += self.__socket.recv(count-len(buffer))
               if bufferSize == len(buffer):
                   raise DCERPCException("The remote host closed the connection")
                   break
               bufferSize = len(buffer)
        else:
            buffer = self.__socket.recv(8192)
        return buffer