[svn-commits] mmichelson: trunk r627 - in /trunk/channels: ./ ooh323c/src/

SVN commits to the Digium repositories svn-commits at lists.digium.com
Wed Jun 4 12:35:29 CDT 2008


Author: mmichelson
Date: Wed Jun  4 12:35:29 2008
New Revision: 627

URL: http://svn.digium.com/view/asterisk-addons?view=rev&rev=627
Log:
------------------------------------------------------------------------
r620 | mmichelson | 2008-06-04 11:55:58 -0500 (Wed, 04 Jun 2008) | 15 lines

A few changes:

1. Add error-checking to the call to ooReadAndProcessStackCommands in oochannels.c
2. (1.2 only) There was a char * being used before being set. This was causing an almost
   immediate crash when ooh323 would load. This commit fixes that issue.
3. Most importantly, fix a security issue wherein arbitrary data could be sent to ooh323's
   command stack listening socket causing a remote crash. See AST-2008-009 for more details.

(closes issue #12756)
Reported by: tzafrir
Patches:
      ooh323_pipev3.diff uploaded by putnopvut (license 60)
Tested by: putnopvut


------------------------------------------------------------------------

Modified:
    trunk/channels/chan_ooh323.c
    trunk/channels/ooh323c/src/ooCapability.c
    trunk/channels/ooh323c/src/ooCmdChannel.c
    trunk/channels/ooh323c/src/ooCmdChannel.h
    trunk/channels/ooh323c/src/ooSocket.c
    trunk/channels/ooh323c/src/oochannels.c
    trunk/channels/ooh323c/src/ooh323ep.c
    trunk/channels/ooh323c/src/ooh323ep.h
    trunk/channels/ooh323c/src/ootrace.c
    trunk/channels/ooh323c/src/ootrace.h

Modified: trunk/channels/chan_ooh323.c
URL: http://svn.digium.com/view/asterisk-addons/trunk/channels/chan_ooh323.c?view=diff&rev=627&r1=626&r2=627
==============================================================================
--- trunk/channels/chan_ooh323.c (original)
+++ trunk/channels/chan_ooh323.c Wed Jun  4 12:35:29 2008
@@ -254,7 +254,7 @@
 
 	/* Don't hold a h323 pvt lock while we allocate a channel */
 	ast_mutex_unlock(&i->lock);
-	ch = ast_channel_alloc(1, state, i->callerid_num, i->callerid_name, i->accountcode, i->exten, i->context, i->amaflags, "OOH323/%s-%08x", host, i & 0xffffffff);
+	ch = ast_channel_alloc(1, state, i->callerid_num, i->callerid_name, i->accountcode, i->exten, i->context, i->amaflags, "OOH323/%s-%08x", host, i);
 	ast_mutex_lock(&i->lock);
 
 	if (ch) {
@@ -430,7 +430,6 @@
 	char tmp[256];
 	char formats[512];
 	int oldformat;
-	char *sport = NULL;
 	int port = 0;
 
 	if (gH323Debug)
@@ -2466,12 +2465,6 @@
 			return 1;
 		}
 
-		/* Create command Listener */
-		if (ooH323EpCreateCmdListener(0) != OO_OK) {
-			ast_log(LOG_ERROR, "OOH323 Command Listener creation failure. OOH323 DISABLED\n");
-			ooH323EpDestroy();
-			return 1;
-		}
 		if (ooh323c_start_stack_thread() < 0) {
 			ast_log(LOG_ERROR, "Failed to start OOH323 stack thread. OOH323 DISABLED\n");
 			ooH323EpDestroy();

Modified: trunk/channels/ooh323c/src/ooCapability.c
URL: http://svn.digium.com/view/asterisk-addons/trunk/channels/ooh323c/src/ooCapability.c?view=diff&rev=627&r1=626&r2=627
==============================================================================
--- trunk/channels/ooh323c/src/ooCapability.c (original)
+++ trunk/channels/ooh323c/src/ooCapability.c Wed Jun  4 12:35:29 2008
@@ -1224,7 +1224,7 @@
                                                               videoCap, dir);
    default:
      OOTRACEDBGC3("ooCapabilityCheckCompatibility_Video - Unsupported video "
-                  "capability. (%s, $s)\n", call->callType, call->callToken);
+                  "capability. (%s, %s)\n", call->callType, call->callToken);
    }
    return FALSE;
 }
@@ -1251,7 +1251,7 @@
    case T_H245DataType_data:
    default:
       OOTRACEDBGC3("ooCapabilityCheckCompatibility - Unsupported  "
-                  "capability. (%s, $s)\n", call->callType, call->callToken);
+                  "capability. (%s, %s)\n", call->callType, call->callToken);
    }
 
    return FALSE;

Modified: trunk/channels/ooh323c/src/ooCmdChannel.c
URL: http://svn.digium.com/view/asterisk-addons/trunk/channels/ooh323c/src/ooCmdChannel.c?view=diff&rev=627&r1=626&r2=627
==============================================================================
--- trunk/channels/ooh323c/src/ooCmdChannel.c (original)
+++ trunk/channels/ooh323c/src/ooCmdChannel.c Wed Jun  4 12:35:29 2008
@@ -27,104 +27,48 @@
 extern OOH323EndPoint gH323ep;
 
 OOSOCKET gCmdChan = 0;
-char gCmdIP[20];
-int gCmdPort = OO_DEFAULT_CMDLISTENER_PORT;
-
-int ooCreateCmdListener()
-{
-   int ret=0;
-   OOIPADDR ipaddrs;
-
-   OOTRACEINFO3("Creating CMD listener at %s:%d\n", 
-                  gH323ep.signallingIP, gH323ep.cmdPort);
-   if((ret=ooSocketCreate (&gH323ep.cmdListener))!=ASN_OK)
-   {
-      OOTRACEERR1("ERROR: Failed to create socket for CMD listener\n");
-      return OO_FAILED;
-   }
-   ret= ooSocketStrToAddr (gH323ep.signallingIP, &ipaddrs);
-   if((ret=ooSocketBind (gH323ep.cmdListener, ipaddrs, 
-                         gH323ep.cmdPort))==ASN_OK) 
-   {
-      ooSocketListen(gH323ep.cmdListener,5); /*listen on socket*/
-      OOTRACEINFO1("CMD listener creation - successful\n");
-      return OO_OK;
-   }
-   else
-   {
-      OOTRACEERR1("ERROR:Failed to create CMD listener\n");
-      return OO_FAILED;
-   }
-
-   return OO_OK;
-}
+pthread_mutex_t gCmdChanLock;
 
 int ooCreateCmdConnection()
 {
-   int ret=0;
-   if((ret=ooSocketCreate (&gCmdChan))!=ASN_OK)
-   {
+   int ret = 0;
+   int thePipe[2];
+
+   if ((ret = pipe(thePipe)) == -1) {
       return OO_FAILED;
    }
-   else
-   {
-      /*
-         bind socket to a port before connecting. Thus avoiding
-         implicit bind done by a connect call. Avoided on windows as 
-         windows sockets have problem in reusing the addresses even after
-         setting SO_REUSEADDR, hence in windows we just allow os to bind
-         to any random port.
-      */
-#ifndef _WIN32
-      ret = ooBindPort(OOTCP,gCmdChan, gCmdIP); 
-#else
-      ret = ooBindOSAllocatedPort(gCmdChan, gCmdIP);
-#endif
-      
-      if(ret == OO_FAILED)
-      {
-         return OO_FAILED;
-      }
+   pthread_mutex_init(&gCmdChanLock);
 
-
-      if((ret=ooSocketConnect(gCmdChan, gCmdIP,
-                           gCmdPort)) != ASN_OK)
-         return OO_FAILED;
-
-   }
+   gH323ep.cmdSock = dup(thePipe[0]);
+   close(thePipe[0]);
+   gCmdChan = dup(thePipe[1]);
+   close(thePipe[1]);
    return OO_OK;
 }
 
 
 int ooCloseCmdConnection()
 {
-   ooSocketClose(gH323ep.cmdSock);
+   close(gH323ep.cmdSock);
    gH323ep.cmdSock = 0;
+   close(gCmdChan);
+   gCmdChan = 0;
+   pthread_mutex_destroy(&gCmdChanLock);
 
-   return OO_OK;
-}
-
-int ooAcceptCmdConnection()    
-{
-   int ret;
-
-   ret = ooSocketAccept (gH323ep.cmdListener, &gH323ep.cmdSock, 
-                         NULL, NULL);
-   if(ret != ASN_OK)
-   {
-      OOTRACEERR1("Error:Accepting CMD connection\n");
-      return OO_FAILED;
-   }
-   OOTRACEINFO1("Cmd connection accepted\n");   
    return OO_OK;
 }
 
 int ooWriteStackCommand(OOStackCommand *cmd)
 {
-   if(ASN_OK != ooSocketSend(gCmdChan, (char*)cmd, sizeof(OOStackCommand)))
-      return OO_FAILED;
+
+	pthread_mutex_lock(&gCmdChanLock);
+   	if (write(gCmdChan, (char*)cmd, sizeof(OOStackCommand)) == -1) {
+		pthread_mutex_unlock(&gCmdChanLock);
+		return OO_FAILED;
+	}
+	pthread_mutex_unlock(&gCmdChanLock);
    
-   return OO_OK;
+   	return OO_OK;
 }
 
 
@@ -135,7 +79,7 @@
    int i, recvLen = 0;
    OOStackCommand cmd;
    memset(&cmd, 0, sizeof(OOStackCommand));
-   recvLen = ooSocketRecv (gH323ep.cmdSock, buffer, MAXMSGLEN);
+   recvLen = read(gH323ep.cmdSock, buffer, MAXMSGLEN);
    if(recvLen <= 0)
    {
       OOTRACEERR1("Error:Failed to read CMD message\n");

Modified: trunk/channels/ooh323c/src/ooCmdChannel.h
URL: http://svn.digium.com/view/asterisk-addons/trunk/channels/ooh323c/src/ooCmdChannel.h?view=diff&rev=627&r1=626&r2=627
==============================================================================
--- trunk/channels/ooh323c/src/ooCmdChannel.h (original)
+++ trunk/channels/ooh323c/src/ooCmdChannel.h Wed Jun  4 12:35:29 2008
@@ -45,23 +45,6 @@
  */
 
 /**
- * This function creates a command listener. This is nothing but a tcp socket
- * in listen mode waiting for incoming connection on which the stack thread 
- * will receive commands.
- *
- * @return          OO_OK, on success; OO_FAILED, on failure
- */
-EXTERN int ooCreateCmdListener();
-
-/**
- * This function is used to accept an incoming connection request for command 
- * channel.
- * 
- * @return          OO_OK, on success; OO_FAILED, on failure
- */
-EXTERN  int ooAcceptCmdConnection();
-
-/**
  * This function is used to setup a command connection with the main stack 
  * thread. The application commands are sent over this connection to stack 
  * thread.

Modified: trunk/channels/ooh323c/src/ooSocket.c
URL: http://svn.digium.com/view/asterisk-addons/trunk/channels/ooh323c/src/ooSocket.c?view=diff&rev=627&r1=626&r2=627
==============================================================================
--- trunk/channels/ooh323c/src/ooSocket.c (original)
+++ trunk/channels/ooh323c/src/ooSocket.c Wed Jun  4 12:35:29 2008
@@ -338,7 +338,7 @@
    if (pNewSocket == 0) return ASN_E_INVPARAM;
 
    *pNewSocket = accept (socket, (struct sockaddr *) (void*) &m_addr, 
-                         &addr_length);
+                         (socklen_t *) &addr_length);
    if (*pNewSocket <= 0) return ASN_E_INVSOCKET;
 
    if (destAddr != 0) 

Modified: trunk/channels/ooh323c/src/oochannels.c
URL: http://svn.digium.com/view/asterisk-addons/trunk/channels/ooh323c/src/oochannels.c?view=diff&rev=627&r1=626&r2=627
==============================================================================
--- trunk/channels/ooh323c/src/oochannels.c (original)
+++ trunk/channels/ooh323c/src/oochannels.c Wed Jun  4 12:35:29 2008
@@ -492,12 +492,6 @@
          *nfds = *((int*)gH323ep.listener);
    }
 
-   if(gH323ep.cmdListener)
-   {
-      FD_SET(gH323ep.cmdListener, pReadfds);
-      if(*nfds < (int)gH323ep.cmdListener)
-         *nfds = (int)gH323ep.cmdListener;
-   }
    if(gH323ep.cmdSock)
    {
       FD_SET(gH323ep.cmdSock, pReadfds);
@@ -582,21 +576,12 @@
       }
    }
 
-
-   if(gH323ep.cmdListener)
-   {
-      if(FD_ISSET(gH323ep.cmdListener, pReadfds))
-      {
-         if(ooAcceptCmdConnection() != OO_OK){
-            OOTRACEERR1("Error:Failed to accept command connection\n");
-            return OO_FAILED;
-         }
-      }
-   }
-
    if(gH323ep.cmdSock) {
       if(FD_ISSET(gH323ep.cmdSock, pReadfds)) {
-         ooReadAndProcessStackCommand();
+         if(ooReadAndProcessStackCommand() != OO_OK) {
+			 /* ooReadAndProcessStackCommand prints an error message */
+			 return OO_FAILED;
+		 }
       }
    }
 

Modified: trunk/channels/ooh323c/src/ooh323ep.c
URL: http://svn.digium.com/view/asterisk-addons/trunk/channels/ooh323c/src/ooh323ep.c?view=diff&rev=627&r1=626&r2=627
==============================================================================
--- trunk/channels/ooh323c/src/ooh323ep.c (original)
+++ trunk/channels/ooh323c/src/ooh323ep.c Wed Jun  4 12:35:29 2008
@@ -24,8 +24,6 @@
 ooEndPoint gH323ep;
 
 
-extern int gCmdPort;
-extern char gCmdIP[20];
 extern DList g_TimerList;
 
 int ooH323EpInitialize
@@ -127,9 +125,7 @@
    ooSetTraceThreshold(OOTRCLVLINFO);
    OO_SETFLAG(gH323ep.flags, OO_M_ENDPOINTCREATED);
 
-   gH323ep.cmdListener = 0;
    gH323ep.cmdSock = 0;
-   gH323ep.cmdPort = OO_DEFAULT_CMDLISTENER_PORT;
    return OO_OK;
 }
 
@@ -145,7 +141,6 @@
    if(localip)
    {
       strcpy(gH323ep.signallingIP, localip);
-      strcpy(gCmdIP, localip);
       OOTRACEINFO2("Signalling IP address is set to %s\n", localip);
    }
    
@@ -154,19 +149,6 @@
       gH323ep.listenPort = listenport;
       OOTRACEINFO2("Listen port number is set to %d\n", listenport);
    }
-   return OO_OK;
-}
-
-int ooH323EpCreateCmdListener(int cmdPort)
-{
-   if(cmdPort != 0)
-   {
-      gH323ep.cmdPort = cmdPort;
-      gCmdPort = cmdPort;
-   }
-   if(ooCreateCmdListener() != OO_OK)
-      return OO_FAILED;
-
    return OO_OK;
 }
 
@@ -373,13 +355,7 @@
          gH323ep.listener = NULL;   
       }
 
-      if(gH323ep.cmdListener != 0)
-      {
-         ooSocketClose(gH323ep.cmdListener);
-         gH323ep.cmdListener = 0;
-      }
-
-      ooGkClientDestroy();  
+	  ooGkClientDestroy();  
 
       if(gH323ep.fptraceFile)
       {

Modified: trunk/channels/ooh323c/src/ooh323ep.h
URL: http://svn.digium.com/view/asterisk-addons/trunk/channels/ooh323c/src/ooh323ep.h?view=diff&rev=627&r1=626&r2=627
==============================================================================
--- trunk/channels/ooh323c/src/ooh323ep.h (original)
+++ trunk/channels/ooh323c/src/ooh323ep.h Wed Jun  4 12:35:29 2008
@@ -147,9 +147,7 @@
 
    OOInterface *ifList; /* interface list for the host we are running on*/
    OOBOOL isGateway;
-   OOSOCKET cmdListener;
    OOSOCKET cmdSock;
-  int cmdPort; /* default 7575 */
 } OOH323EndPoint;
 
 #define ooEndPoint OOH323EndPoint
@@ -165,19 +163,6 @@
  */
 EXTERN int ooH323EpInitialize
    (enum OOCallMode callMode, const char* tracefile);
-
-
-
-/**
- * This function is used to create a command listener for the endpoint.
- * Before any command is issued to the endpoint, command listener must be
- * created.
- * @param cmdPort          Port number on which command listener waits for
- *                         incoming requests.
- * 
- * @return                 OO_OK, on success; OO_FAILED, on failure
- */
-EXTERN int ooH323EpCreateCmdListener(int cmdPort);
 
 /**
  * This function is used to represent the H.323 application endpoint as 

Modified: trunk/channels/ooh323c/src/ootrace.c
URL: http://svn.digium.com/view/asterisk-addons/trunk/channels/ooh323c/src/ootrace.c?view=diff&rev=627&r1=626&r2=627
==============================================================================
--- trunk/channels/ooh323c/src/ootrace.c (original)
+++ trunk/channels/ooh323c/src/ootrace.c Wed Jun  4 12:35:29 2008
@@ -70,7 +70,7 @@
    
 #else
    struct tm *ptime;
-   char dateString[10];
+   char dateString[15];
    time_t t = time(NULL);
    ptime = localtime(&t);
    strftime(timeString, 100, "%H:%M:%S", ptime);
@@ -100,7 +100,7 @@
    if(printDate)
    {
       printDate = 0;
-      strftime(dateString, 10, "%D", ptime);
+      strftime(dateString, 15, "%m/%d/%Y", ptime);
       fprintf(gH323ep.fptraceFile, "---------Date %s---------\n", 
               dateString);
    }

Modified: trunk/channels/ooh323c/src/ootrace.h
URL: http://svn.digium.com/view/asterisk-addons/trunk/channels/ooh323c/src/ootrace.h?view=diff&rev=627&r1=626&r2=627
==============================================================================
--- trunk/channels/ooh323c/src/ootrace.h (original)
+++ trunk/channels/ooh323c/src/ootrace.h Wed Jun  4 12:35:29 2008
@@ -52,7 +52,7 @@
 #define TRACELVL 1
 #endif
 
-#define OOTRACEERR1(a)        ooTrace(OOTRCLVLERR,a)
+#define OOTRACEERR1(a)        ooTrace(OOTRCLVLERR,"%s", a)
 #define OOTRACEERR2(a,b)      ooTrace(OOTRCLVLERR,a,b)
 #define OOTRACEERR3(a,b,c)    ooTrace(OOTRCLVLERR,a,b,c)
 #define OOTRACEERR4(a,b,c,d)  ooTrace(OOTRCLVLERR,a,b,c,d)
@@ -72,7 +72,7 @@
 #define OOTRACEDBGA3(a,b,c)   ooTrace(OOTRCLVLDBGA,a,b,c)
 #define OOTRACEDBGA4(a,b,c,d) ooTrace(OOTRCLVLDBGA,a,b,c,d)
 #define OOTRACEDBGA5(a,b,c,d,e) ooTrace(OOTRCLVLDBGA,a,b,c,d,e)
-#define OOTRACEDBGB1(a)       ooTrace(OOTRCLVLDBGB,a)
+#define OOTRACEDBGB1(a)       ooTrace(OOTRCLVLDBGB,"%s",a)
 #define OOTRACEDBGB2(a,b)     ooTrace(OOTRCLVLDBGB,a,b)
 #define OOTRACEDBGB3(a,b,c)   ooTrace(OOTRCLVLDBGB,a,b,c)
 #define OOTRACEDBGB4(a,b,c,d) ooTrace(OOTRCLVLDBGB,a,b,c,d)
@@ -129,7 +129,7 @@
  *
  * @return            - none
  */
-EXTERN void ooTrace(OOUINT32 traceLevel, const char * fmtspec, ...);
+EXTERN void ooTrace(OOUINT32 traceLevel, const char * fmtspec, ...)__attribute__((format(printf, 2, 3)));
 
 /**
  * Helper function for the trace function. This function performs actual




More information about the svn-commits mailing list