* Bug #12296 fixed - Call to getenv with a big environment variable led to memory... 64/10464/8
Alexandre HERISSE [Wed, 13 Feb 2013 13:00:40 +0000 (14:00 +0100)]
Now getenvc functions returns expected size when output buffer is NULL

Change-Id: I767d5a91dea142db6c55e90ce1d211229066dd24

scilab/CHANGES_5.4.X
scilab/modules/core/src/c/getversion.c
scilab/modules/core/src/c/inisci-c.c
scilab/modules/dynamic_link/src/c/dynamic_link.c
scilab/modules/io/sci_gateway/c/sci_getenv.c
scilab/modules/io/src/c/getenvc.c
scilab/modules/io/tests/nonreg_tests/bug_12296.dia.ref [new file with mode: 0644]
scilab/modules/io/tests/nonreg_tests/bug_12296.tst [new file with mode: 0644]

index 5a3d8f7..b84811d 100644 (file)
@@ -363,6 +363,9 @@ Bug fixes
 
 * Bug #12285 fixed - export_to_hdf5 function could not handle any matrix in append mode.
 
+* Bug #12296 fixed - Call to getenv with a big environment variable
+                     led to memory corruption.
+
 * Bug #12304 fixed - Undock, redock and undock led to an exception.
 
 
index 601016b..e9eb162 100644 (file)
@@ -22,7 +22,6 @@
 #include "loadversion.h"
 #include "freeArrayOfString.h"
 #include "MALLOC.h"
-#include "../../../io/includes/getenvc.h"
 /*--------------------------------------------------------------------------*/
 #define TCLSCI_MODULE_NAME "tclsci"
 #define TCLTK_OPTION_STRING "tk"
index 22482b6..3686648 100644 (file)
@@ -43,15 +43,16 @@ int SetSci(void)
        {
                C2F(getenvc)(&ierr,"SCI",buf,&lbuf,&iflag);
 
-               if ( ierr== 1) 
+               if (ierr != 0)
                {
-               #ifdef  _MSC_VER
-               MessageBox(NULL,gettext("SCI environment variable not defined.\n"),gettext("Warning"),MB_ICONWARNING);
-               #else
-               fprintf(stderr, "%s", _("SCI environment variable not defined.\n"));
-               #endif
-               exit(1);
+#ifdef  _MSC_VER
+                   MessageBox(NULL,gettext("SCI environment variable not defined.\n"),gettext("Warning"),MB_ICONWARNING);
+#else
+                   fprintf(stderr, "%s", _("SCI environment variable not defined.\n"));
+#endif
+               exit(1);
                }
+
                setSCIpath(buf);
                FREE(buf);
                buf = NULL;
@@ -129,7 +130,7 @@ int C2F(gettmpdir)(char *buf,int *nbuf,long int lbuf)
 {
        int ierr,iflag=0,l1buf=lbuf;
        C2F(getenvc)(&ierr,"TMPDIR",buf,&l1buf,&iflag);
-       if ( ierr== 1) 
+       if (ierr != 0) 
        {
 #ifdef  _MSC_VER
                MessageBox(NULL,gettext("TMPDIR not defined.\n"),gettext("Warning"),MB_ICONWARNING);
@@ -138,6 +139,7 @@ int C2F(gettmpdir)(char *buf,int *nbuf,long int lbuf)
 #endif
                exit(1);
        }
+
        *nbuf = (int)strlen(buf);
        return 0;
 }
index 5b91e50..64d1bac 100644 (file)
@@ -28,7 +28,6 @@
 #include "FileExist.h"
 #include "ilib_verbose.h"
 #ifdef _MSC_VER
-#include "getenvc.h"
 #include "dllinfo.h"
 #endif
 #include "getshortpathname.h"
index e76f9f8..a457039 100644 (file)
@@ -42,6 +42,9 @@ int sci_getenv(char *fname,unsigned long fname_len)
        char *pStVarTwo = NULL;
        int lenStVarTwo = 0;
 
+    int iflag = 0;
+    int m_out = 1, n_out = 1;
+
        Rhs = Max(Rhs,0);
 
        CheckRhs(1,2);
@@ -159,35 +162,24 @@ int sci_getenv(char *fname,unsigned long fname_len)
                return 0;
        }
 
-       #ifdef _MSC_VER
-       length_env = _MAX_ENV;
-       #else
-       length_env = bsiz;
-       #endif
-
-       default_env_value =  pStVarTwo;
-       env_name = pStVarOne;
-
-       env_value = (char*)MALLOC( (length_env + 1) *sizeof(char) );
+    default_env_value =  pStVarTwo;
+    env_name = pStVarOne;
 
-       if (env_value == NULL)
-       {
-               if (default_env_value) {FREE(default_env_value); default_env_value = NULL;}
-               if (env_name) {FREE(env_name); env_name = NULL;}
-
-               Scierror(999,_("%s: No more memory.\n"), fname);
-               return 0;
-       }
-       else
-       {
-               int m_out = 1, n_out = 1;
-               int iflag = 0;
+    C2F(getenvc)(&ierr, env_name, NULL, &length_env, &iflag);
 
-               C2F(getenvc)(&ierr, env_name, env_value, &length_env, &iflag);
+    if(ierr == 0)
+    {
+           env_value = (char*)MALLOC( (length_env + 1) * sizeof(char) );
+           if (env_value == NULL)
+           {
+                   Scierror(999,_("%s: No more memory.\n"), fname);
+           }
+           else
+        {
+                   C2F(getenvc)(&ierr, env_name, env_value, &length_env, &iflag);
 
-               if (ierr == 0)
-               {
-                       sciErr = createMatrixOfString(pvApiCtx, Rhs + 1, m_out, n_out, &env_value);
+            //create variable on stack and return it.
+                       sciErr = createMatrixOfString(pvApiCtx, Rhs + 1, m_out, n_out, (const char**)&env_value);
                        if(sciErr.iErr)
                        {
                                printError(&sciErr, 0);
@@ -197,33 +189,50 @@ int sci_getenv(char *fname,unsigned long fname_len)
 
                        LhsVar(1) = Rhs + 1;
                        PutLhsVar();    
-               }
-               else
+        }
+    }
+    else //ierr == 1 -> env var does not exist.
+    {
+               if (default_env_value)
                {
-                       if (default_env_value)
-                       {
-                               sciErr = createMatrixOfString(pvApiCtx, Rhs + 1, m_out, n_out, &default_env_value);
-                               if(sciErr.iErr)
-                               {
-                                       printError(&sciErr, 0);
-                    Scierror(999,_("%s: Memory allocation error.\n"), fname);
-                                       return 0;
-                               }
-
-                               LhsVar(1) = Rhs + 1;
-                               PutLhsVar();
-                       }
-                       else
+                       sciErr = createMatrixOfString(pvApiCtx, Rhs + 1, m_out, n_out, &default_env_value);
+                       if(sciErr.iErr)
                        {
-                               Scierror(999,_("%s: Undefined environment variable %s.\n"), fname, env_name);
+                               printError(&sciErr, 0);
+                Scierror(999,_("%s: Memory allocation error.\n"), fname);
                        }
+            else
+            {
+                           LhsVar(1) = Rhs + 1;
+                           PutLhsVar();
+                   }
+        }
+               else
+               {
+                       Scierror(999,_("%s: Undefined environment variable %s.\n"), fname, env_name);
                }
+    }
 
-               if (default_env_value) {FREE(default_env_value); default_env_value = NULL;}
-               if (env_name) {FREE(env_name); env_name = NULL;}
-               if (env_value) {FREE(env_value); env_value = NULL;}
 
-       }
+
+       if (default_env_value) 
+    {
+        FREE(default_env_value);
+        default_env_value = NULL;
+    }
+
+       if (env_name)
+    {
+        FREE(env_name);
+        env_name = NULL;
+    }
+       
+    if (env_value)
+    {
+        FREE(env_value);
+        env_value = NULL;
+    }
+
        return 0;
 }
 /*--------------------------------------------------------------------------*/
index bae872b..d14061e 100644 (file)
 /*--------------------------------------------------------------------------*/
 #ifndef _MSC_VER
 static void searchenv_others(const char *filename, const char *varname,
-                                                        char *pathname);
+                             char *pathname);
 #endif
 /*--------------------------------------------------------------------------*/
-void C2F(getenvc)(int *ierr,char *var,char *buf,int *buflen,int *iflag)
+void C2F(getenvc)(int *ierr, char *var, char *buf, int *buflen, int *iflag)
 {
 #ifdef _MSC_VER
 
 
-       BOOL bMalloc = FALSE;
-       wchar_t *wvar = to_wide_string(var);
-       wchar_t *wbuf = _wgetenv(wvar);
-
-       *ierr = 0;
-       if(wbuf == NULL)
-       {
-               bMalloc = TRUE;
-               wbuf = (wchar_t*)MALLOC(sizeof(wchar_t) * *buflen);
-               if (GetEnvironmentVariableW(wvar, wbuf,(DWORD)*buflen) == 0)
-               {
-                       if( *iflag == 1 )
-                       {
-                               sciprint(_("Undefined environment variable %s.\n"),var);
-                       }
-                       *ierr=1;
-               }
-       }
-
-       if(*ierr != 1)
-       {
-               char* temp = wide_string_to_UTF8(wbuf);
-               strcpy(buf, temp);
-               *buflen = (int)strlen(buf);
-               *ierr=0;
-       }
-
-       if(bMalloc)
-       {
-               FREE(wbuf);
-       }
+    BOOL bMalloc = FALSE;
+    wchar_t *wvar = to_wide_string(var);
+    wchar_t *wbuf = _wgetenv(wvar);
+
+    *ierr = 0;
+    if (wbuf == NULL)
+    {
+        DWORD iLen = GetEnvironmentVariableW(wvar, NULL, 0);
+        bMalloc = TRUE;
+        if (iLen == 0)
+        {
+            if ( *iflag == 1 )
+            {
+                sciprint(_("Undefined environment variable %s.\n"), var);
+            }
+            *ierr = 1;
+        }
+        else
+        {
+            *buflen = iLen;
+            wbuf = (wchar_t*)MALLOC(sizeof(wchar_t) * *buflen);
+            if (GetEnvironmentVariableW(wvar, wbuf, (DWORD)*buflen) == 0)
+            {
+                if ( *iflag == 1 )
+                {
+                    sciprint(_("Undefined environment variable %s.\n"), var);
+                }
+                *ierr = 1;
+            }
+        }
+    }
+
+    if (*ierr != 1)
+    {
+        *buflen = (int)wcslen(wbuf);
+        if (buf)
+        {
+            char* temp = wide_string_to_UTF8(wbuf);
+            strcpy(buf, temp);
+            *ierr = 0;
+        }
+    }
+
+    if (bMalloc)
+    {
+        FREE(wbuf);
+    }
 #else
-       char *locale = NULL;
-       locale=getenv(var);
-       if ( locale == NULL )
-       {
-               if ( *iflag == 1 ) sciprint(_("Undefined environment variable %s.\n"),var);
-               *ierr=1;
-       }
-       else
-       {
-               *buflen = (int)strlen(locale);
-               strcpy(buf,locale);
-               *ierr=0;
-       }
+    char *locale = NULL;
+    locale = getenv(var);
+    if ( locale == NULL )
+    {
+        if ( *iflag == 1 )
+        {
+            sciprint(_("Undefined environment variable %s.\n"), var);
+        }
+        *ierr = 1;
+    }
+    else
+    {
+        // updating the size of char array "buf"
+        *buflen = (int)strlen(locale);
+        if (buf)
+        {
+            // to avoid buffer overflow, we check the size of the source buffer
+            // and the size of the destination buffer
+            if ((int) strlen(locale) <= *buflen)
+            {
+                // "locale" can be copied entirely to "buf"
+                strcpy(buf, locale);
+                *ierr = 0;
+            }
+            else
+            {
+                *ierr = 2;
+            }
+        }
+    }
 #endif
 }
 /*--------------------------------------------------------------------------*/
 #ifndef _MSC_VER
 static void searchenv_others(const char *filename,
-                                                        const char *varname,
-                                                        char *pathname)
+                             const char *varname,
+                             char *pathname)
 {
-       char *cp = NULL;
-
-       *pathname = '\0';
-
-       if( filename[0] == DIR_SEPARATOR[0])
-       {
-               strcpy(pathname, filename);
-               return;
-       }
-
-       cp = getenv(varname);
-       if(cp == NULL)
-       {
-               /* environment Variable not defined. */
-               return;
-       }
-
-       while(*cp)
-       {
-               char *concat = NULL;
-               *pathname = '\0';
-               concat = pathname;
-               /* skip PATH_SEPARATOR[0] and empty entries */
-               while( (*cp) && (*cp == PATH_SEPARATOR[0]) )
-               {
-                       cp++;
-               }
-
-               /* copy path */
-               while( (*cp) && (*cp != PATH_SEPARATOR[0]) )
-               {
-                       *concat = *cp;
-                       cp++;
-                       concat++;
-               }
-
-               if ( concat == pathname )
-               {
-                       /* filename not found */
-                       *pathname = '\0';
-                       return;
-               }
-
-               if( *(concat-1) != DIR_SEPARATOR[0] )
-               {
-                       /* add directory separator */
-                       *concat = DIR_SEPARATOR[0];
-                       concat++;
-               }
-
-               /* concatate path & filename */
-               strcpy(concat, filename);
-
-               /* file exists ? */
-               if(FileExist(pathname))
-               {
-                       // file found
-                       return;
-               }
-       }
-
-       /* file not found */
-       *pathname = '\0';
+    char *cp = NULL;
+
+    *pathname = '\0';
+
+    if ( filename[0] == DIR_SEPARATOR[0])
+    {
+        strcpy(pathname, filename);
+        return;
+    }
+
+    cp = getenv(varname);
+    if (cp == NULL)
+    {
+        /* environment Variable not defined. */
+        return;
+    }
+
+    while (*cp)
+    {
+        char *concat = NULL;
+        *pathname = '\0';
+        concat = pathname;
+        /* skip PATH_SEPARATOR[0] and empty entries */
+        while ( (*cp) && (*cp == PATH_SEPARATOR[0]) )
+        {
+            cp++;
+        }
+
+        /* copy path */
+        while ( (*cp) && (*cp != PATH_SEPARATOR[0]) )
+        {
+            *concat = *cp;
+            cp++;
+            concat++;
+        }
+
+        if ( concat == pathname )
+        {
+            /* filename not found */
+            *pathname = '\0';
+            return;
+        }
+
+        if ( *(concat - 1) != DIR_SEPARATOR[0] )
+        {
+            /* add directory separator */
+            *concat = DIR_SEPARATOR[0];
+            concat++;
+        }
+
+        /* concatate path & filename */
+        strcpy(concat, filename);
+
+        /* file exists ? */
+        if (FileExist(pathname))
+        {
+            // file found
+            return;
+        }
+    }
+
+    /* file not found */
+    *pathname = '\0';
 }
 #endif
 /*--------------------------------------------------------------------------*/
-char *searchEnv(const char *name,const char *env_var)
+char *searchEnv(const char *name, const char *env_var)
 {
-       char *buffer = NULL;
-       char fullpath[PATH_MAX];
+    char *buffer = NULL;
+    char fullpath[PATH_MAX];
 
-       strcpy(fullpath,"");
+    strcpy(fullpath, "");
 
 #if _MSC_VER
-       {
-               wchar_t *wname                  = NULL;
-               wchar_t *wenv_var               = NULL;
-               wchar_t wfullpath[PATH_MAX];
+    {
+        wchar_t *wname                 = NULL;
+        wchar_t *wenv_var              = NULL;
+        wchar_t wfullpath[PATH_MAX];
 
-               wname                   = to_wide_string((char*)name);
-               wenv_var        = to_wide_string((char*)env_var);
+        wname                  = to_wide_string((char*)name);
+        wenv_var       = to_wide_string((char*)env_var);
 
-               wcscpy(wfullpath,L"");
+        wcscpy(wfullpath, L"");
 
-               _wsearchenv(wname, wenv_var, wfullpath);
+        _wsearchenv(wname, wenv_var, wfullpath);
 
-               if (wcslen(wfullpath) > 0)
-               {
-                       buffer = wide_string_to_UTF8(wfullpath);
-               }
+        if (wcslen(wfullpath) > 0)
+        {
+            buffer = wide_string_to_UTF8(wfullpath);
+        }
 
-               FREE(wname);
-               FREE(wenv_var);
-       }
+        FREE(wname);
+        FREE(wenv_var);
+    }
 #else
-       searchenv_others(name, env_var,fullpath);
-       if (strlen(fullpath) > 0)
-       {
-               buffer = strdup(fullpath);
-       }
+    searchenv_others(name, env_var, fullpath);
+    if (strlen(fullpath) > 0)
+    {
+        buffer = strdup(fullpath);
+    }
 #endif
-       return buffer;
+    return buffer;
 }
 /*--------------------------------------------------------------------------*/
 
diff --git a/scilab/modules/io/tests/nonreg_tests/bug_12296.dia.ref b/scilab/modules/io/tests/nonreg_tests/bug_12296.dia.ref
new file mode 100644 (file)
index 0000000..fc8750a
--- /dev/null
@@ -0,0 +1,24 @@
+// =============================================================================
+// Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
+// Copyright (C) 2008 - Scilab Enterprises - Alexandre HERISSE
+//
+//  This file is distributed under the same license as the Scilab package.
+// =============================================================================
+//
+// <-- Non-regression test for bug 12296 -->
+//
+// <-- Bugzilla URL -->
+// http://bugzilla.scilab.org/show_bug.cgi?id=12296
+//
+// <-- Short Description -->
+// call to getenv with an big environment variable leads to memory corruption
+a = "azerty"
+ a  =
+ azerty   
+b = "";
+for i = 1:4000
+    b = b + a;
+end
+assert_checktrue(setenv("test", b));
+assert_checkequal(getenv("test"), b);
diff --git a/scilab/modules/io/tests/nonreg_tests/bug_12296.tst b/scilab/modules/io/tests/nonreg_tests/bug_12296.tst
new file mode 100644 (file)
index 0000000..e903a67
--- /dev/null
@@ -0,0 +1,24 @@
+// =============================================================================
+// Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
+// Copyright (C) 2008 - Scilab Enterprises - Alexandre HERISSE
+//
+//  This file is distributed under the same license as the Scilab package.
+// =============================================================================
+//
+// <-- Non-regression test for bug 12296 -->
+//
+// <-- Bugzilla URL -->
+// http://bugzilla.scilab.org/show_bug.cgi?id=12296
+//
+// <-- Short Description -->
+// call to getenv with an big environment variable leads to memory corruption
+
+a = "azerty"
+b = "";
+for i = 1:4000
+    b = b + a;
+end
+
+assert_checktrue(setenv("test", b));
+assert_checkequal(getenv("test"), b);
+