freerdp/core/gcc: channel name hardening
authorNorbert Federa <norbert.federa@thincast.com>
Mon, 30 May 2016 12:40:23 +0000 (14:40 +0200)
committerNorbert Federa <norbert.federa@thincast.com>
Mon, 30 May 2016 12:40:23 +0000 (14:40 +0200)
According to [MS-RDPBCGR 2.2.1.3.4.1 Channel Definition Structure]
the channel name must be an 8-byte array containing a null-terminated
collection of seven ANSI characters that uniquely identify the channel.

We did not check if the transmitted name was null-terminated which
could have the usual severe effects on stabiliy and security since
the channel name is used in several functions expecting a null-
terminated string (strlen, printf, etc.)

libfreerdp/core/gcc.c

index e19e019..36c9f18 100644 (file)
@@ -1428,8 +1428,18 @@ BOOL gcc_read_client_network_data(wStream* s, rdpMcs* mcs, UINT16 blockLength)
        /* channelDefArray */
        for (i = 0; i < mcs->channelCount; i++)
        {
-               /* CHANNEL_DEF */
+               /**
+                * CHANNEL_DEF
+                * - name: an 8-byte array containing a null-terminated collection
+                *   of seven ANSI characters that uniquely identify the channel.
+                * - options: a 32-bit, unsigned integer. Channel option flags
+                */
                Stream_Read(s, mcs->channels[i].Name, 8); /* name (8 bytes) */
+               if (!memchr(mcs->channels[i].Name, 0, 8))
+               {
+                       WLog_ERR(TAG, "protocol violation: received a static channel name with missing null-termination");
+                       return FALSE;
+               }
                Stream_Read_UINT32(s, mcs->channels[i].options); /* options (4 bytes) */
                mcs->channels[i].ChannelId = mcs->baseChannelId++;
        }