Repository: microsoft/terminal (language: cpp)

Issue:
ConIoSrvComm::Connect memory leak
The ConIoSrvComm::Connect function has a memory leak from using the RtlCreateUnicodeString function incorrectly:

https://github.com/microsoft/terminal/blob/396cbbb151ccd6fc9ae0356771ce1a5af5623e48/src/interactivity/onecore/ConIoSrvComm.cpp#L101-L106

RtlCreateUnicodeString creates a copy of the string on the process heap and the PortName variable has local-scope. The string doesn't get freed with RtlFreeUnicodeString before the function returns creating a memory leak.

CIS_ALPC_PORT_NAME is a constant string and the PortName variable should instead be initialized using the RTL_CONSTANT_STRING macro:

`static UNICODE_STRING PortName = RTL_CONSTANT_STRING(CIS_ALPC_PORT_NAME);`

It could alternatively use RtlInitUnicodeString but since the string is hard-coded it should be using the macro.






Discussion / hints:
Hey, good find! It's nice to have someone looking at this code, especially given that it doesn't even build in the solution today :smile:

Fix patch:
diff --git a/src/interactivity/onecore/ConIoSrvComm.cpp b/src/interactivity/onecore/ConIoSrvComm.cpp
index cdeebed38c3..e38b677efac 100644
--- a/src/interactivity/onecore/ConIoSrvComm.cpp
+++ b/src/interactivity/onecore/ConIoSrvComm.cpp
@@ -71,12 +71,11 @@ ConIoSrvComm::~ConIoSrvComm()
 
 [[nodiscard]] NTSTATUS ConIoSrvComm::Connect()
 {
-    BOOL Ret = TRUE;
     NTSTATUS Status = STATUS_SUCCESS;
 
     // Port handle and name.
     HANDLE PortHandle;
-    UNICODE_STRING PortName;
+    static UNICODE_STRING PortName = RTL_CONSTANT_STRING(CIS_ALPC_PORT_NAME);
 
     // Generic Object Manager attributes for the port object and ALPC-specific
     // port attributes.
@@ -98,13 +97,6 @@ ConIoSrvComm::~ConIoSrvComm()
     // Structure used to iterate over the handles given to us by the server.
     ALPC_MESSAGE_HANDLE_INFORMATION HandleInfo;
 
-    // Initialize the server port name.
-    Ret = RtlCreateUnicodeString(&PortName, CIS_ALPC_PORT_NAME);
-    if (!Ret)
-    {
-        return STATUS_NO_MEMORY;
-    }
-
     // Initialize the attributes of the port object.
     InitializeObjectAttributes(&ObjectAttributes,
                                NULL,


Test patch: