More fixes from FlawFinder: tighter checks on valid file names and a few extra checks otherwise.

meteoio/FileUtils.cc

@@ -22,14 +22,16 @@
 
 #if defined _WIN32 || defined __MINGW32__
 	#ifndef NOMINMAX
-        #define NOMINMAX
-    #endif
+		#define NOMINMAX
+	#endif
 	#include <windows.h>
 	#include "Shlwapi.h"
 #else
 	#include <dirent.h>
 	#include <sys/stat.h>
 	#include <unistd.h>
+	#include <errno.h>
+	#include <cstring>
 #endif
 
 #include <meteoio/FileUtils.h>
@@ -42,9 +44,11 @@ void copy_file(const std::string& src, const std::string& dest)
 {
 	if (src == dest) return; //copying to the same file doesn't make sense, but is no crime either
 
+	if (!IOUtils::fileExists(src)) throw FileNotFoundException(src, AT);
 	std::ifstream fin(src.c_str(), std::ios::binary);
 	if (fin.fail()) throw FileAccessException(src, AT);
 
+	if (!IOUtils::validFileName(dest)) throw InvalidFileNameException(dest, AT);
 	std::ofstream fout(dest.c_str(), std::ios::binary);
 	if (fout.fail()) {
 		fin.close();
@@ -59,7 +63,7 @@ void copy_file(const std::string& src, const std::string& dest)
 
 std::string cleanPath(std::string in_path, const bool& resolve)
 {
-	if(!resolve) { //do not resolve links, relative paths, etc
+	if (!resolve) { //do not resolve links, relative paths, etc
 		std::replace(in_path.begin(), in_path.end(), '\\', '/');
 		return in_path;
 	} else {
@@ -68,21 +72,23 @@ std::string cleanPath(std::string in_path, const bool& resolve)
 		char **ptr = NULL;
 		char *out_buff = (char*)calloc(MAX_PATH, sizeof(char));
 		const DWORD status = GetFullPathName(in_path.c_str(), MAX_PATH, out_buff, ptr);
-		if(status!=0 && status<=MAX_PATH) in_path = out_buff;
+		if (status!=0 && status<=MAX_PATH) in_path = out_buff;
 		free(out_buff);
 
 		std::replace(in_path.begin(), in_path.end(), '\\', '/');
 		return in_path;
 	#else //POSIX
 		std::replace(in_path.begin(), in_path.end(), '\\', '/');
-
+		
 		char *real_path = realpath(in_path.c_str(), NULL); //POSIX 2008
-		if(real_path!=NULL) {
+		if (real_path!=NULL) {
 			const std::string tmp(real_path);
 			free(real_path);
 			return tmp;
-		} else
+		} else {
+			std::cerr << "Path expansion of \'" << in_path << "\' failed. Reason:\t" << strerror(errno) << "\n";
 			return in_path; //something failed in realpath, keep it as it is
+		}
 	#endif
 	}
 }
@@ -130,13 +136,20 @@ std::string getFilename(const std::string& path)
 		return path;
 }
 
+//this checks for path+file
 bool validFileName(const std::string& filename)
 {
+#if defined _WIN32 || defined __MINGW32__ || defined __CYGWIN__
 	const size_t startpos = filename.find_first_not_of(" \t\n"); // Find the first character position after excluding leading blank spaces
-	if ((startpos!=0) || (filename==".") || (filename=="..")) {
+	const size_t invalid_char = filename.find_first_of("\000*:<>?|"); //find possible invalid characters
+#else
+	const size_t startpos = filename.find_first_not_of(" \t\n"); // Find the first character position after excluding leading blank spaces
+	const size_t invalid_char = filename.find_first_of("\000"); //find possible invalid characters
+#endif
+	
+	if ((startpos!=0) || (invalid_char!=std::string::npos) || (filename==".") || (filename=="..")) {
 		return false;
 	}
-
 	return true;
 }
 

meteoio/dataClasses/Coords.cc

@@ -958,7 +958,7 @@ lat, double& lon)
 	char lat_str[len]=""; //each string must be able to accomodate the whole length to avoid buffer overflow
 	char lon_str[len]="";
 
-	if(coordinates.size()>len) {
+	if(coordinates.size()>=len) {
 		throw InvalidFormatException("Given lat/lon string is too long! ",AT);
 	}
 

meteoio/plugins/ARCIO.cc

@@ -170,12 +170,8 @@ void ARCIO::read2DGrid_internal(Grid2DObject& grid_out, const std::string& full_
 	std::string line;
 	std::map<std::string, std::string> header; // A map to save key value pairs of the file header
 
-	if (!IOUtils::validFileName(full_name)) {
-		throw InvalidFileNameException(full_name, AT);
-	}
-	if (!IOUtils::fileExists(full_name)) {
-		throw FileNotFoundException(full_name, AT);
-	}
+	if (!IOUtils::validFileName(full_name)) throw InvalidFileNameException(full_name, AT);
+	if (!IOUtils::fileExists(full_name)) throw FileNotFoundException(full_name, AT);
 
 	fin.clear();
 	fin.open (full_name.c_str(), ifstream::in);

meteoio/plugins/PNGIO.cc

@@ -768,10 +768,10 @@ void PNGIO::writeMetadata(png_structp &png_ptr, png_infop &info_ptr)
 	for(size_t ii=0; ii<nr; ii++) {
 		key[ii] = (char *)calloc(sizeof(char), max_len);
 		text[ii] = (char *)calloc(sizeof(char), max_len);
-		strncpy(key[ii], metadata_key[ii].c_str(), max_len);
-		strncpy(text[ii], metadata_text[ii].c_str(), max_len);
-		info_text[ii].key = key[ii];
-		info_text[ii].text = text[ii];
+		strncpy(key[ii], metadata_key[ii].c_str(), max_len-1); //in case the '\0' was not counted by maxlen
+		strncpy(text[ii], metadata_text[ii].c_str(), max_len-1);
+		info_text[ii].key = key[ii]+'\0'; //strncpy does not always '\0' terminate
+		info_text[ii].text = text[ii]+'\0';
 		info_text[ii].compression = PNG_TEXT_COMPRESSION_NONE;
 	}
 

meteoio/plugins/libsmet.cc

@@ -20,6 +20,12 @@
 #include <string.h>
 #include <limits>
 
+#if defined _WIN32 || defined __MINGW32__
+	#include <windows.h>
+#else
+	#include <sys/stat.h>
+#endif
+
 using namespace std;
 
 namespace smet {
@@ -79,6 +85,33 @@ bool SMETCommon::initStaticData()
 	return true;
 }
 
+#if defined _WIN32 || defined __MINGW32__
+bool fileExists(const std::string& filename)
+{
+	const DWORD attributes = GetFileAttributes( filename.c_str() );
+	
+	if (attributes==INVALID_FILE_ATTRIBUTES || attributes==FILE_ATTRIBUTE_VIRTUAL 
+	     || attributes==FILE_ATTRIBUTE_DIRECTORY || attributes==FILE_ATTRIBUTE_DEVICE)
+		return false;
+	
+	return true;
+}
+#else
+bool fileExists(const std::string& filename)
+{
+	struct stat buffer ;
+
+	if ((stat( filename.c_str(), &buffer))!=0) {//File exists if stat returns 0
+		return false;
+	}
+	
+	if (S_ISREG(buffer.st_mode) || S_ISFIFO(buffer.st_mode) || S_ISLNK(buffer.st_mode))
+		return true;
+	else
+		return false; //exclude char device, block device, sockets, etc
+}
+#endif
+
 double SMETCommon::convert_to_double(const std::string& in_string)
 {
 	char* conversion_end = NULL;
@@ -659,6 +692,7 @@ SMETReader::SMETReader(const std::string& in_fname)
 {
 	std::ifstream fin; //Input file streams
 	fin.clear();
+	if (!SMETCommon::fileExists(filename)) throw SMETException(filename, AT); //prevent invalid filenames
 	fin.open (filename.c_str(), ios::in|ios::binary); //ascii does end of line translation, which messes up the pointer code
 	if (fin.fail()) {
 		ostringstream ss;
@@ -964,6 +998,7 @@ void SMETReader::read(std::vector<std::string>& vec_timestamp, std::vector<doubl
 
 	ifstream fin;
 	fin.clear();
+	if (!SMETCommon::fileExists(filename)) throw SMETException(filename, AT); //prevent invalid filenames
 	fin.open (filename.c_str(), ios::in|ios::binary); //ascii mode messes up pointer code on windows (automatic eol translation)
 	if (fin.fail()) {
 		ostringstream ss;
@@ -1011,10 +1046,9 @@ void SMETReader::read(std::vector<double>& vec_data)
 		throw SMETException("Requesting not to read timestamp when there is one present in \""+filename+"\"", SMET_AT);
 
 	ios_base::openmode mode = ios::in|ios::binary; //read as binary to avoid eol mess
-	/*if (!isAscii)
-		mode = ios::in | ios::binary;*/
 
 	ifstream fin;
+	if (!SMETCommon::fileExists(filename)) throw SMETException(filename, AT); //prevent invalid filenames
 	fin.open (filename.c_str(), mode);
 	if (fin.fail()) {
 		ostringstream ss;

meteoio/plugins/libsmet.h

@@ -63,6 +63,7 @@ class SMETException : public std::exception {
  */
 class SMETCommon {
 	public:
+		static bool fileExists(const std::string& filename);
 		static double convert_to_double(const std::string& in_string);
 		static int convert_to_int(const std::string& in_string);
 		static void stripComments(std::string& str);