Fri Jan 5 21:46:16 UTC 2007 Wallace Zhang <zhang_w@ociweb.com>

2 files changed, 60 insertions, 7 deletions

diff --git a/ACE/ChangeLog b/ACE/ChangeLog
index 67cebbf83fa..40665ddbdd4 100644
--- a/ACE/ChangeLog
+++ b/ACE/ChangeLog
@@ -1,3 +1,12 @@
+Fri Jan  5 21:46:16 UTC 2007  Wallace Zhang  <zhang_w@ociweb.com>
+
+        * ace/Process.cpp:
+
+         Improved the implementation of ACE_Process_Options::setenv(...)
+         to address the potential buffer overflow when invoking
+         sprintf and/or vsprintf. This fixes bug 2660 and 2762. More
+         discussions have been accumulated in RT 9198.
+
 Fri Jan  5 19:06:17 UTC 2007  Chad Elliott  <elliott_c@ociweb.com>
 
         * bin/msvc_static_order.lst:
diff --git a/ACE/ace/Process.cpp b/ACE/ace/Process.cpp
index 8f100b8b293..b79fbcbd0f9 100644
--- a/ACE/ace/Process.cpp
+++ b/ACE/ace/Process.cpp
@@ -18,6 +18,7 @@
 #include "ace/OS_NS_unistd.h"
 #include "ace/OS_Memory.h"
 #include "ace/Countdown_Time.h"
+#include "ace/Truncate.h"
 
 #if defined (ACE_VXWORKS) && (ACE_VXWORKS > 0x600) && defined (__RTP__)
 # include <rtpLib.h>
@@ -841,29 +842,72 @@ int
 ACE_Process_Options::setenv (const ACE_TCHAR *variable_name,
                              const ACE_TCHAR *format, ...)
 {
-  ACE_TCHAR newformat[DEFAULT_COMMAND_LINE_BUF_LEN];
+  // To address the potential buffer overflow,
+  // we now allocate the buffer on heap with a variable size.
+  size_t const buflen = ACE_OS::strlen (variable_name) + ACE_OS::strlen (format) + 2;
+  ACE_TCHAR *newformat = 0;
+  ACE_NEW_RETURN (newformat, ACE_TCHAR[buflen], -1);
+  ACE_Auto_Basic_Array_Ptr<ACE_TCHAR> safe_newformat (newformat);
 
   // Add in the variable name.
-  ACE_OS::sprintf (newformat,
+  ACE_OS::sprintf (safe_newformat.get (),
                    ACE_LIB_TEXT ("%s=%s"),
                    variable_name,
                    format);
 
-  ACE_TCHAR stack_buf[DEFAULT_COMMAND_LINE_BUF_LEN];
-
   // Start varargs.
   va_list argp;
   va_start (argp, format);
 
   // Add the rest of the varargs.
-  ACE_OS::vsprintf (stack_buf, newformat, argp);
+  size_t tmp_buflen = DEFAULT_COMMAND_LINE_BUF_LEN > buflen 
+                      ? static_cast<size_t> (DEFAULT_COMMAND_LINE_BUF_LEN) : buflen;
+  int retval = 0;
+
+  ACE_TCHAR *stack_buf = 0;
+  ACE_NEW_RETURN (stack_buf, ACE_TCHAR[tmp_buflen], -1);
+  ACE_Auto_Basic_Array_Ptr<ACE_TCHAR> safe_stack_buf (stack_buf);
+
+  do 
+    {
+      retval = ACE_OS::vsnprintf (safe_stack_buf.get (), tmp_buflen, safe_newformat.get (), argp);
+      if (retval > ACE_Utils::Truncate<int> (tmp_buflen))
+        {
+          tmp_buflen *= 2;
+          ACE_NEW_RETURN (stack_buf, ACE_TCHAR[tmp_buflen], -1);
+          safe_stack_buf.reset (stack_buf);
+        }
+      else
+        break;
+    }
+  while (1);
+
+  if (retval == -1)
+    {
+      // In case that vsnprintf is not supported,
+      // e.g., LynxOS and VxWorks 5, we have to
+      // fall back to vsprintf.
+      if (errno == ENOTSUP)
+        { 
+          // ALERT: Since we have to use vsprintf here, there is still a chance that
+          // the stack_buf overflows, i.e., the length of the resulting string
+          // can still possibly go beyond the allocated stack_buf.
+          retval = ACE_OS::vsprintf (safe_stack_buf.get (), safe_newformat.get (), argp);
+          if (retval == -1)
+            // vsprintf is failed.
+            return -1;
+        }
+      else
+        // vsnprintf is failed.
+        return -1;
+    }
 
   // End varargs.
   va_end (argp);
 
   // Append the string to our environment buffer.
-  if (this->setenv_i (stack_buf,
-                      ACE_OS::strlen (stack_buf)) == -1)
+  if (this->setenv_i (safe_stack_buf.get (),
+                      ACE_OS::strlen (safe_stack_buf.get ())) == -1)
     return -1;
 
 #if defined (ACE_WIN32)