Fix VS security warnings using SafeGetEnv and SafeFOpen utility functions.

diff --git a/src/config.h.in b/src/config.h.in
index ed254fe..5d16df0 100644 (file)
--- a/src/config.h.in
+++ b/src/config.h.in
@@ -35,13 +35,5 @@
 // ---------------------------------------------------------------------------
 // Windows port
 #ifdef _WIN32
-// This must be defined before the windows.h is included.
-// It's needed for mutex.h, to give access to the TryLock method.
-#  if !defined(_WIN32_WINNT) && !(defined( __MINGW32__) || defined(__MINGW64__))
-#    define _WIN32_WINNT 0x0400
-#  endif
-#  if defined(_MSC_VER) && !defined(_CRT_SECURE_NO_WARNINGS)
-#    define _CRT_SECURE_NO_WARNINGS
-#  endif
 #  include "windows_port.h"
 #endif

diff --git a/src/gflags.cc b/src/gflags.cc
index 7727327..0f1f0b0 100644 (file)
--- a/src/gflags.cc
+++ b/src/gflags.cc
@@ -992,8 +992,8 @@ static string ReadFileIntoString(const char* filename) {
   const int kBufSize = 8092;
   char buffer[kBufSize];
   string s;
-  FILE* fp = fopen(filename, "r");
-  if (!fp)  PFATAL(filename);
+  FILE* fp;
+  if ((errno = SafeFOpen(&fp, filename, "r") != 0)) PFATAL(filename);
   size_t n;
   while ( (n=fread(buffer, 1, kBufSize, fp)) > 0 ) {
     if (ferror(fp))  PFATAL(filename);
@@ -1137,8 +1137,8 @@ string CommandLineFlagParser::ProcessFromenvLocked(const string& flagval,
     }
 
     const string envname = string("FLAGS_") + string(flagname);
-    const char* envval = getenv(envname.c_str());
-    if (!envval) {
+       string envval;
+       if (!SafeGetEnv(envname.c_str(), envval)) {
       if (errors_are_fatal) {
         error_flags_[flagname] = (string(kError) + envname +
                                   " not found in environment\n");
@@ -1147,15 +1147,14 @@ string CommandLineFlagParser::ProcessFromenvLocked(const string& flagval,
     }
 
     // Avoid infinite recursion.
-    if ((strcmp(envval, "fromenv") == 0) ||
-        (strcmp(envval, "tryfromenv") == 0)) {
+    if (envval == "fromenv" || envval == "tryfromenv") {
       error_flags_[flagname] =
           StringPrintf("%sinfinite recursion on environment flag '%s'\n",
-                       kError, envval);
+                       kError, envval.c_str());
       continue;
     }
 
-    msg += ProcessSingleOptionLocked(flag, envval, set_mode);
+    msg += ProcessSingleOptionLocked(flag, envval.c_str(), set_mode);
   }
   return msg;
 }
@@ -1335,14 +1334,15 @@ string CommandLineFlagParser::ProcessOptionsFromStringLocked(
 
 template<typename T>
 T GetFromEnv(const char *varname, const char* type, T dflt) {
-  const char* const valstr = getenv(varname);
-  if (!valstr)
-    return dflt;
-  FlagValue ifv(new T, type, true);
-  if (!ifv.ParseFrom(valstr))
-    ReportError(DIE, "ERROR: error parsing env variable '%s' with value '%s'\n",
-                varname, valstr);
-  return OTHER_VALUE_AS(ifv, T);
+  std::string valstr;
+  if (SafeGetEnv(varname, valstr)) {
+    FlagValue ifv(new T, type, true);
+    if (!ifv.ParseFrom(valstr.c_str())) {
+      ReportError(DIE, "ERROR: error parsing env variable '%s' with value '%s'\n",
+                  varname, valstr.c_str());
+       }
+    return OTHER_VALUE_AS(ifv, T);
+  } else return dflt;
 }
 
 bool AddFlagValidator(const void* flag_ptr, ValidateFnProto validate_fn_proto) {
@@ -1754,8 +1754,8 @@ bool ReadFlagsFromString(const string& flagfilecontents,
 
 // TODO(csilvers): nix prog_name in favor of ProgramInvocationShortName()
 bool AppendFlagsIntoFile(const string& filename, const char *prog_name) {
-  FILE *fp = fopen(filename.c_str(), "a");
-  if (!fp) {
+  FILE *fp;
+  if (SafeFOpen(&fp, filename.c_str(), "a") != 0) {
     return false;
   }
 
@@ -1813,10 +1813,18 @@ uint64 Uint64FromEnv(const char *v, uint64 dflt) {
 double DoubleFromEnv(const char *v, double dflt) {
   return GetFromEnv(v, "double", dflt);
 }
+
+#ifdef _MSC_VER
+#  pragma warning(push)
+#  pragma warning(disable: 4996) // ignore getenv security warning
+#endif
 const char *StringFromEnv(const char *varname, const char *dflt) {
   const char* const val = getenv(varname);
   return val ? val : dflt;
 }
+#ifdef _MSC_VER
+#  pragma warning(pop)
+#endif
 
 
 // --------------------------------------------------------------------

diff --git a/src/util.h b/src/util.h
index 9d6f8dc..3856365 100644 (file)
--- a/src/util.h
+++ b/src/util.h
@@ -324,6 +324,33 @@ inline std::string StringPrintf(const char* format, ...) {
   return output;
 }
 
+inline bool SafeGetEnv(const char *varname, std::string &valstr)
+{
+#if defined(_MSC_VER) && _MSC_VER >= 1400
+       char  *val;
+       size_t sz;
+       if (_dupenv_s(&val, &sz, varname) != 0 || !val) return false;
+       valstr = val;
+       free(val);
+#else
+       const char * const val = getenv(varname);
+       if (!val) return false;
+       valstr = val;
+#endif
+       return true;
+}
+
+inline errno_t SafeFOpen(FILE **fp, const char* fname, const char *mode)
+{
+#if defined(_MSC_VER) && _MSC_VER >= 1400
+       return fopen_s(fp, fname, mode);
+#else
+       assert(fp != NULL);
+       *fp = fopen(fname, mode);
+       return errno;
+#endif
+}
+
 
 } // namespace GFLAGS_NAMESPACE
 

diff --git a/src/windows_port.cc b/src/windows_port.cc
index 5511f8d..1f40458 100644 (file)
--- a/src/windows_port.cc
+++ b/src/windows_port.cc
@@ -44,12 +44,20 @@
 
 // These call the windows _vsnprintf, but always NUL-terminate.
 #if !defined(__MINGW32__) && !defined(__MINGW64__)  /* mingw already defines */
+
+#ifdef _MSC_VER
+#  pragma warning(push)
+#  pragma warning(disable: 4996) // ignore _vsnprintf security warning
+#endif
 int safe_vsnprintf(char *str, size_t size, const char *format, va_list ap) {
   if (size == 0)        // not even room for a \0?
     return -1;          // not what C99 says to do, but what windows does
-  str[size-1] = '\0';
+  str[size-1] = '\0'; 
   return _vsnprintf(str, size-1, format, ap);
 }
+#ifdef _MSC_VER
+#  pragma warning(pop)
+#endif
 
 int snprintf(char *str, size_t size, const char *format, ...) {
   int r;
@@ -59,4 +67,5 @@ int snprintf(char *str, size_t size, const char *format, ...) {
   va_end(ap);
   return r;
 }
+
 #endif  /* #if !defined(__MINGW32__) && !defined(__MINGW64__) */

diff --git a/src/windows_port.h b/src/windows_port.h
index 6da8796..246f715 100644 (file)
--- a/src/windows_port.h
+++ b/src/windows_port.h
@@ -42,8 +42,14 @@
 
 #include "config.h"
 
+// This must be defined before the windows.h is included.
+// It's needed for mutex.h, to give access to the TryLock method.
+#  if !defined(_WIN32_WINNT) && !(defined( __MINGW32__) || defined(__MINGW64__))
+#    define _WIN32_WINNT 0x0400
+#  endif
+// We always want minimal includes
 #ifndef WIN32_LEAN_AND_MEAN
-#  define WIN32_LEAN_AND_MEAN  /* We always want minimal includes */
+#  define WIN32_LEAN_AND_MEAN
 #endif
 #include <windows.h>
 #include <direct.h>          /* for mkdir */
@@ -65,6 +71,10 @@ extern int GFLAGS_DLL_DECL safe_vsnprintf(char *str, size_t size,
 #define va_copy(dst, src)  (dst) = (src)
 #endif  /* #if !defined(__MINGW32__) && !defined(__MINGW64__) */
 
+#ifdef _MSC_VER
+#  pragma warning(push)
+#  pragma warning(disable: 4996) // ignore getenv security warning
+#endif
 inline void setenv(const char* name, const char* value, int) {
   // In windows, it's impossible to set a variable to the empty string.
   // We handle this by setting it to "0" and the NUL-ing out the \0.
@@ -86,6 +96,9 @@ inline void setenv(const char* name, const char* value, int) {
       *getenv(name) = '\0';            // works when putenv() copies nameval
   }
 }
+#ifdef _MSC_VER
+#  pragma warning(pop)
+#endif
 
 #define strcasecmp _stricmp
 

diff --git a/test/gflags_unittest.cc b/test/gflags_unittest.cc
index 4990182..185d562 100644 (file)
--- a/test/gflags_unittest.cc
+++ b/test/gflags_unittest.cc
@@ -1098,7 +1098,8 @@ TEST(DeprecatedFunctionsTest, AppendFlagsIntoFile) {
   const bool r = AppendFlagsIntoFile(filename, "not the real argv0");
   EXPECT_TRUE(r);
 
-  FILE* fp = fopen(filename.c_str(), "r");
+  FILE* fp;
+  EXPECT_EQ(0, SafeFOpen(&fp, filename.c_str(), "r"));
   EXPECT_TRUE(fp != NULL);
   char line[8192];
   EXPECT_TRUE(fgets(line, sizeof(line)-1, fp) != NULL);  // get the first line
@@ -1134,7 +1135,8 @@ TEST(DeprecatedFunctionsTest, ReadFromFlagsFile) {
 TEST(DeprecatedFunctionsTest, ReadFromFlagsFileFailure) {
   FLAGS_test_int32 = -20;
   string filename(TmpFile("flagfile3"));
-  FILE* fp = fopen(filename.c_str(), "w");
+  FILE* fp;
+  EXPECT_EQ(0, SafeFOpen(&fp, filename.c_str(), "w"));
   EXPECT_TRUE(fp != NULL);
   // Note the error in the bool assignment below...
   fprintf(fp, "%s\n--test_int32=-21\n--test_bool=not_a_bool!\n", GetArgv0());