remove use of str(n)cpy and sscanf

- considered unsecure by MS
coin-or · Dec 11, 2024 · 9a99518 · 9a99518
1 parent 5dc7322
commit 9a99518
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 80 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -19,6 +19,7 @@ More detailed information about incremental changes can be found in the
   an MPI-enabled build of MUMPS [#790, by Alex Tyler Chapman].
 - Updated build system to current autotools versions; initial support for icx/ifx and flang
 - Removed use of `vsprintf` and `sprintf`. Added `IpoptData::Append_info_string(std::string,double)`.
+- Removed use of `strcpy`, `strncpy`, `strdup`, and `sscanf`.
 
 ### 3.14.16 (2024-04-22)
 

diff --git a/src/Algorithm/Inexact/IpIterativePardisoSolverInterface.cpp b/src/Algorithm/Inexact/IpIterativePardisoSolverInterface.cpp
@@ -221,8 +221,9 @@ bool IterativePardisoSolverInterface::InitializeImpl(
       int num_procs = 1;
       if( var != NULL )
       {
-         sscanf(var, "%d", &num_procs);
-         if( num_procs < 1 )
+         char* endptr;
+         num_procs = strtol(var, &endptr, 10);
+         if( *endptr != '\0' || num_procs < 1 )
          {
             Jnlst().Printf(J_ERROR, J_LINEAR_ALGEBRA,
                            "Invalid value for OMP_NUM_THREADS (\"%s\").\n", var);
@@ -395,18 +396,15 @@ void write_iajaa_matrix(
    {
       /* Write header */
       char mat_name[128];
-      char mat_pref[32];
+      const char* mat_pref;
 
       Index NNZ = ia[N] - 1;
       Index i;
 
-      if( getenv("IPOPT_WRITE_PREFIX") )
+      mat_pref = getenv("IPOPT_WRITE_PREFIX");
+      if( mat_pref == NULL )
       {
-         strcpy(mat_pref, getenv("IPOPT_WRITE_PREFIX"));
-      }
-      else
-      {
-         strcpy(mat_pref, "mat-ipopt");
+         mat_pref = "mat-ipopt";
       }
 
       Snprintf(mat_name, 127, "%s_%03d-%02d.iajaa", mat_pref, iter_cnt, sol_cnt);
@@ -444,18 +442,15 @@ void write_iajaa_matrix(
    {
       /* Write header */
       char mat_name[128];
-      char mat_pref[32];
+      const char* mat_pref;
 
       Index i;
       Index j;
 
-      if( getenv("IPOPT_WRITE_PREFIX") )
-      {
-         strcpy(mat_pref, getenv("IPOPT_WRITE_PREFIX"));
-      }
-      else
+      mat_pref = getenv("IPOPT_WRITE_PREFIX");
+      if( mat_pref == NULL )
       {
-         strcpy(mat_pref, "mat-ipopt");
+         mat_pref = "mat-ipopt";
       }
 
       Snprintf(mat_name, 127, "%s_%03d-%02d.mtx", mat_pref, iter_cnt, sol_cnt);

diff --git a/src/Algorithm/LinearSolvers/IpPardisoMKLSolverInterface.cpp b/src/Algorithm/LinearSolvers/IpPardisoMKLSolverInterface.cpp
@@ -360,18 +360,15 @@ void write_iajaa_matrix(
    {
       /* Write header */
       char mat_name[128];
-      char mat_pref[32];
+      const char* mat_pref;
 
       Index NNZ = ia[N] - 1;
       Index i;
 
-      if( getenv("IPOPT_WRITE_PREFIX") )
+      mat_pref = getenv("IPOPT_WRITE_PREFIX");
+      if( mat_pref == NULL )
       {
-         strcpy(mat_pref, getenv("IPOPT_WRITE_PREFIX"));
-      }
-      else
-      {
-         strcpy(mat_pref, "mat-ipopt");
+         mat_pref = "mat-ipopt";
       }
 
       Snprintf(mat_name, 127, "%s_%03d-%02d.iajaa", mat_pref, iter_cnt, sol_cnt);
@@ -409,18 +406,15 @@ void write_iajaa_matrix(
    {
       /* Write header */
       char mat_name[128];
-      char mat_pref[32];
+      const char* mat_pref;
 
       Index i;
       Index j;
 
-      if( getenv("IPOPT_WRITE_PREFIX") )
-      {
-         strcpy(mat_pref, getenv("IPOPT_WRITE_PREFIX"));
-      }
-      else
+      mat_pref = getenv("IPOPT_WRITE_PREFIX");
+      if( mat_pref == NULL)
       {
-         strcpy(mat_pref, "mat-ipopt");
+         mat_pref = "mat-ipopt";
       }
 
       Snprintf(mat_name, 127, "%s_%03d-%02d.mtx", mat_pref, iter_cnt, sol_cnt);

diff --git a/src/Algorithm/LinearSolvers/IpPardisoSolverInterface.cpp b/src/Algorithm/LinearSolvers/IpPardisoSolverInterface.cpp
@@ -406,8 +406,9 @@ bool PardisoSolverInterface::InitializeImpl(
       char* var = getenv("OMP_NUM_THREADS");
       if( var != NULL )
       {
-         sscanf(var, "%d", &num_procs);
-         if( num_procs < 1 )
+         char* endptr;
+         num_procs = strtol(var, &endptr, 10);
+         if( *endptr != '\0' || num_procs < 1 )
          {
             Jnlst().Printf(J_ERROR, J_LINEAR_ALGEBRA,
                            "Invalid value for OMP_NUM_THREADS (\"%s\").\n", var);
@@ -600,18 +601,15 @@ void write_iajaa_matrix(
    {
       /* Write header */
       char mat_name[128];
-      char mat_pref[32];
+      const char* mat_pref;
 
       Index NNZ = ia[N] - 1;
       Index i;
 
-      if( getenv("IPOPT_WRITE_PREFIX") )
+      mat_pref = getenv("IPOPT_WRITE_PREFIX");
+      if( mat_pref == NULL )
       {
-         strcpy(mat_pref, getenv("IPOPT_WRITE_PREFIX"));
-      }
-      else
-      {
-         strcpy(mat_pref, "mat-ipopt");
+         mat_pref = "mat-ipopt";
       }
 
       Snprintf(mat_name, 127, "%s_%03d-%02d.iajaa", mat_pref, iter_cnt, sol_cnt);
@@ -649,18 +647,15 @@ void write_iajaa_matrix(
    {
       /* Write header */
       char mat_name[128];
-      char mat_pref[32];
+      const char* mat_pref;
 
       Index i;
       Index j;
 
-      if( getenv("IPOPT_WRITE_PREFIX") )
-      {
-         strcpy(mat_pref, getenv("IPOPT_WRITE_PREFIX"));
-      }
-      else
+      mat_pref = getenv("IPOPT_WRITE_PREFIX");
+      if( mat_pref == NULL )
       {
-         strcpy(mat_pref, "mat-ipopt");
+         mat_pref = "mat-ipopt";
       }
 
       Snprintf(mat_name, 127, "%s_%03d-%02d.mtx", mat_pref, iter_cnt, sol_cnt);

diff --git a/src/Apps/AmplSolver/AmplTNLP.cpp b/src/Apps/AmplSolver/AmplTNLP.cpp
@@ -14,13 +14,6 @@
 
 #include <cstring>
 
-// strdup is named _strdup on Windows
-#ifdef _WIN32
-#define ipopt_strdup _strdup
-#else
-#define ipopt_strdup strdup
-#endif
-
 /* AMPL includes */
 #include "asl.h"
 #include "asl_pfgh.h"
@@ -1083,7 +1076,7 @@ void AmplTNLP::write_solution_file(
    // We need to copy the message into a non-const char array to make
    // it work with the AMPL C function.
    char* cmessage = new char[message.length() + 1];
-   strcpy(cmessage, message.c_str());
+   memcpy(cmessage, message.c_str(), message.length() + 1);
 
    write_sol(cmessage, x_sol_, lambda_sol_, (Option_Info* )Oinfo_ptr_);
 
@@ -1320,8 +1313,8 @@ AmplOptionsList::AmplOption::AmplOption(
    : ipopt_option_name_(ipopt_option_name),
      type_(type)
 {
-   description_ = new char[description.size() + 1];
-   strcpy(description_, description.c_str());
+   description_ = new char[description.length() + 1];
+   memcpy(description_, description.c_str(), description.length() + 1);
 }
 
 AmplOptionsList::~AmplOptionsList()
@@ -1370,8 +1363,8 @@ void* AmplOptionsList::Keywords(
    for( std::map<std::string, SmartPtr<const AmplOption> >::iterator iter = ampl_options_map_.begin();
         iter != ampl_options_map_.end(); ++iter )
    {
-      keywords[ioption].name = new char[iter->first.size() + 1];
-      strcpy(keywords[ioption].name, iter->first.c_str());
+      keywords[ioption].name = new char[iter->first.length() + 1];
+      memcpy(keywords[ioption].name, iter->first.c_str(), iter->first.length() + 1);
       keywords[ioption].desc = iter->second->Description();
       switch( iter->second->Type() )
       {
@@ -1704,6 +1697,7 @@ AmplTNLP::get_options(
    const char* sname;
    const char* bsname;
    const char* opname;
+   size_t len;
    if( ampl_option_string )
    {
       opname = ampl_option_string;
@@ -1731,12 +1725,15 @@ AmplTNLP::get_options(
 
    DBG_ASSERT(!Oinfo_ptr_);
    Option_Info* Oinfo = new Option_Info;
-   Oinfo->sname = new char[strlen(sname) + 1];
-   strcpy(Oinfo->sname, sname);
-   Oinfo->bsname = new char[strlen(bsname) + 1];
-   strcpy(Oinfo->bsname, bsname);
-   Oinfo->opname = new char[strlen(opname) + 1];
-   strcpy(Oinfo->opname, opname);
+   len = strlen(sname);
+   Oinfo->sname = new char[len + 1];
+   memcpy(Oinfo->sname, sname, len + 1);
+   len = strlen(bsname);
+   Oinfo->bsname = new char[len + 1];
+   memcpy(Oinfo->bsname, bsname, len + 1);
+   len = strlen(opname);
+   Oinfo->opname = new char[len + 1];
+   memcpy(Oinfo->opname, opname, len + 1);
    Oinfo->keywds = keywds;
    Oinfo->n_keywds = (int)n_options;
    // Set the default for the remaining entries
@@ -1814,7 +1811,8 @@ void AmplSuffixHandler::PrepareAmplForSuffixes(
    suftab_ = new SufDecl[n];
    for( Index i = 0; i < n; i++ )
    {
-      suftab_[i].name = ipopt_strdup(suffix_ids_[i].c_str());
+      suftab_[i].name = new char[suffix_ids_[i].length() + 1];
+      memcpy(suftab_[i].name, suffix_ids_[i].c_str(), suffix_ids_[i].length() + 1);
       suftab_[i].table = 0;
 
       if( suffix_sources_[i] == Variable_Source )

diff --git a/src/Common/IpOptionsList.cpp b/src/Common/IpOptionsList.cpp
@@ -539,25 +539,21 @@ bool OptionsList::GetNumericValue(
    {
       // Some people like to use 'd' instead of 'e' in floating point
       // numbers.  Therefore, we change a 'd' to an 'e'
-      char* buffer = new char[strvalue.length() + 1];
-      strcpy(buffer, strvalue.c_str());
       for( size_t i = 0; i < strvalue.length(); ++i )
       {
-         if( buffer[i] == 'd' || buffer[i] == 'D' )
+         if( strvalue[i] == 'd' || strvalue[i] == 'D' )
          {
-            buffer[i] = 'e';
+            strvalue[i] = 'e';
          }
       }
       char* p_end;
-      Number retval = strtod(buffer, &p_end);
+      Number retval = strtod(strvalue.c_str(), &p_end);
       if( *p_end != '\0' && !isspace(*p_end) )
       {
-         delete[] buffer;
          std::string msg = "Option \"" + tag + "\": Double value expected, but non-numeric value \"" + strvalue
                            + "\" found.\n";
          THROW_EXCEPTION(OPTION_INVALID, msg);
       }
-      delete[] buffer;
       value = retval;
       return true;
    }
@@ -746,25 +742,21 @@ bool OptionsList::ReadFromStream(
          {
             // Some people like to use 'd' instead of 'e' in floating
             // point numbers.  Therefore, we change a 'd' to an 'e'
-            char* buffer = new char[value.length() + 1];
-            strcpy(buffer, value.c_str());
             for( size_t i = 0; i < value.length(); ++i )
             {
-               if( buffer[i] == 'd' || buffer[i] == 'D' )
+               if( value[i] == 'd' || value[i] == 'D' )
                {
-                  buffer[i] = 'e';
+                  value[i] = 'e';
                }
             }
             char* p_end;
-            Number retval = strtod(buffer, &p_end);
+            Number retval = strtod(value.c_str(), &p_end);
             if( *p_end != '\0' && !isspace(*p_end) )
             {
-               delete[] buffer;
                std::string msg = "Option \"" + tag + "\": Double value expected, but non-numeric option value \""
                                  + value + "\" found.\n";
                THROW_EXCEPTION(OPTION_INVALID, msg);
             }
-            delete [] buffer;
             bool result = SetNumericValue(tag, retval, allow_clobber);
             ASSERT_EXCEPTION(result, OPTION_INVALID,
                              "Error setting numeric value read from file.");

diff --git a/src/Interfaces/IpStdFInterface.c b/src/Interfaces/IpStdFInterface.c
@@ -416,7 +416,7 @@ static char* f2cstr(
    cstr = (char*) malloc(sizeof(char) * (len + 1));
    if( cstr != NULL )
    {
-      strncpy(cstr, FSTR, len);
+      memcpy(cstr, FSTR, len);
       cstr[len] = '\0';
    }