[asterisk-addons-commits] mmichelson: branch 1.2 r620 - in /branches/1.2/asterisk-ooh323c: ooh323c/src/...

SVN commits to the Asterisk addons project asterisk-addons-commits at lists.digium.com
Wed Jun 4 11:55:59 CDT 2008


Author: mmichelson
Date: Wed Jun  4 11:55:58 2008
New Revision: 620

URL: http://svn.digium.com/view/asterisk-addons?view=rev&rev=620
Log:
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:
    branches/1.2/asterisk-ooh323c/ooh323c/src/ooCmdChannel.c
    branches/1.2/asterisk-ooh323c/ooh323c/src/ooCmdChannel.h
    branches/1.2/asterisk-ooh323c/ooh323c/src/oochannels.c
    branches/1.2/asterisk-ooh323c/ooh323c/src/ooh323ep.c
    branches/1.2/asterisk-ooh323c/ooh323c/src/ooh323ep.h
    branches/1.2/asterisk-ooh323c/src/chan_h323.c

Modified: branches/1.2/asterisk-ooh323c/ooh323c/src/ooCmdChannel.c
URL: http://svn.digium.com/view/asterisk-addons/branches/1.2/asterisk-ooh323c/ooh323c/src/ooCmdChannel.c?view=diff&rev=620&r1=619&r2=620
==============================================================================
--- branches/1.2/asterisk-ooh323c/ooh323c/src/ooCmdChannel.c (original)
+++ branches/1.2/asterisk-ooh323c/ooh323c/src/ooCmdChannel.c Wed Jun  4 11:55:58 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: branches/1.2/asterisk-ooh323c/ooh323c/src/ooCmdChannel.h
URL: http://svn.digium.com/view/asterisk-addons/branches/1.2/asterisk-ooh323c/ooh323c/src/ooCmdChannel.h?view=diff&rev=620&r1=619&r2=620
==============================================================================
--- branches/1.2/asterisk-ooh323c/ooh323c/src/ooCmdChannel.h (original)
+++ branches/1.2/asterisk-ooh323c/ooh323c/src/ooCmdChannel.h Wed Jun  4 11:55:58 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: branches/1.2/asterisk-ooh323c/ooh323c/src/oochannels.c
URL: http://svn.digium.com/view/asterisk-addons/branches/1.2/asterisk-ooh323c/ooh323c/src/oochannels.c?view=diff&rev=620&r1=619&r2=620
==============================================================================
--- branches/1.2/asterisk-ooh323c/ooh323c/src/oochannels.c (original)
+++ branches/1.2/asterisk-ooh323c/ooh323c/src/oochannels.c Wed Jun  4 11:55:58 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: branches/1.2/asterisk-ooh323c/ooh323c/src/ooh323ep.c
URL: http://svn.digium.com/view/asterisk-addons/branches/1.2/asterisk-ooh323c/ooh323c/src/ooh323ep.c?view=diff&rev=620&r1=619&r2=620
==============================================================================
--- branches/1.2/asterisk-ooh323c/ooh323c/src/ooh323ep.c (original)
+++ branches/1.2/asterisk-ooh323c/ooh323c/src/ooh323ep.c Wed Jun  4 11:55:58 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: branches/1.2/asterisk-ooh323c/ooh323c/src/ooh323ep.h
URL: http://svn.digium.com/view/asterisk-addons/branches/1.2/asterisk-ooh323c/ooh323c/src/ooh323ep.h?view=diff&rev=620&r1=619&r2=620
==============================================================================
--- branches/1.2/asterisk-ooh323c/ooh323c/src/ooh323ep.h (original)
+++ branches/1.2/asterisk-ooh323c/ooh323c/src/ooh323ep.h Wed Jun  4 11:55:58 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: branches/1.2/asterisk-ooh323c/src/chan_h323.c
URL: http://svn.digium.com/view/asterisk-addons/branches/1.2/asterisk-ooh323c/src/chan_h323.c?view=diff&rev=620&r1=619&r2=620
==============================================================================
--- branches/1.2/asterisk-ooh323c/src/chan_h323.c (original)
+++ branches/1.2/asterisk-ooh323c/src/chan_h323.c Wed Jun  4 11:55:58 2008
@@ -1887,8 +1887,12 @@
    ooconfig.mTCPPortStart = 12030;
    ooconfig.mTCPPortEnd = 12230;
 
+   ast_log(LOG_NOTICE, "Check 1\n");
+
    v = ast_variable_browse(cfg, "general");
    while(v) {
+
+	   ast_log(LOG_NOTICE, "v is %s\n", v->name);
    
       if (!strcasecmp(v->name, "port")) {
          gPort = (int)strtol(v->value, NULL, 10);
@@ -2046,6 +2050,7 @@
          ast_parse_allow_disallow(&gPrefs, &gCapability, tcodecs, 1);
       }
       else if (!strcasecmp(v->name, "dtmfmode")) {
+		  ast_log(LOG_NOTICE, "v's value is %s\n", v->value);
          if (!strcasecmp(v->value, "inband"))
             gDTMFMode=H323_DTMF_INBAND;
          else if (!strcasecmp(v->value, "rfc2833"))
@@ -2070,8 +2075,9 @@
    {
       if(strcasecmp(cat, "general")) 
       {
-         int friend_type = strcasecmp(utype, "friend");
+         int friend_type;
          utype = ast_variable_retrieve(cfg, cat, "type");
+		 friend_type = strcasecmp(utype, "friend");
          if(utype)
          {
             if(!strcmp(utype, "user") || 0 == friend_type)
@@ -2592,7 +2598,6 @@
          ast_log(LOG_ERROR, "Capabilities failure for OOH323. OOH323 Disabled.\n");
          return 1;
       }
-  
 
       /* Create H.323 listener */
       if(ooCreateH323Listener()!= OO_OK)
@@ -2603,15 +2608,7 @@
          ooH323EpDestroy();
          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. "




More information about the asterisk-addons-commits mailing list