Merge pull request #1991 from G10h4ck/extrafiles_overflow_check

ExtraFileHash check for integer overflow

libretroshare/src/ft/ftextralist.cc

@@ -4,7 +4,7 @@
  * libretroshare: retroshare core library                                      *
  *                                                                             *
  * Copyright (C) 2008  Robert Fernie <retroshare@lunamutt.com>                 *
- * Copyright (C) 2018-2019  Gioacchino Mazzurco <gio@eigenlab.org>             *
+ * Copyright (C) 2018-2020  Gioacchino Mazzurco <gio@eigenlab.org>             *
  *                                                                             *
  * This program is free software: you can redistribute it and/or modify        *
  * it under the terms of the GNU Lesser General Public License as              *
@@ -21,6 +21,9 @@
  *                                                                             *
  *******************************************************************************/
 
+#include <limits>
+#include <system_error>
+
 #ifdef WINDOWS_SYS
 #include "util/rswin.h"
 #endif
@@ -245,12 +248,8 @@ bool	ftExtraList::cleanupOldFiles()
 		/* remove items */
 		for(std::list<RsFileHash>::iterator rit = toRemove.begin(); rit != toRemove.end(); ++rit)
         {
-			if (mFiles.end() != (it = mFiles.find(*rit)))
+			if (mFiles.end() != (it = mFiles.find(*rit))) mFiles.erase(it);
-			{
+			mHashOfHash.erase(makeEncryptedHash(*rit));
-				cleanupEntry(it->second.info.path, it->second.info.transfer_info_flags);
-				mFiles.erase(it);
-			}
-			mHashOfHash.erase(makeEncryptedHash(*rit)) ;
         }
 
 		IndicateConfigChanged();
@@ -258,46 +257,39 @@ bool	ftExtraList::cleanupOldFiles()
 	return true;
 }
 
-
-bool	ftExtraList::cleanupEntry(std::string /*path*/, TransferRequestFlags /*flags*/)
-{
-//	if (flags & RS_FILE_CONFIG_CLEANUP_DELETE)
-//	{
-//		/* Delete the file? - not yet! */
-//	}
-	return true;
-}
-
-		/***
-		 * Hash file, and add to the files, 
-		 * file is removed after period.
-		 **/
-
 bool ftExtraList::hashExtraFile(
         std::string path, uint32_t period, TransferRequestFlags flags )
 {
-#ifdef  DEBUG_ELIST
+	constexpr rstime_t max_int = std::numeric_limits<int>::max();
-	std::cerr << "ftExtraList::hashExtraFile() path: " << path;
+	const rstime_t now = time(nullptr);
-	std::cerr << " period: " << period;
+	const rstime_t timeOut = now + period;
-	std::cerr << " flags: " << flags;
 
-	std::cerr << std::endl;
+	if(timeOut > max_int)
-#endif
-
-	auto failure = [](std::string errMsg)
 	{
-		RsErr() << __PRETTY_FUNCTION__ << " " << errMsg << std::endl;
+		/* Under the hood period is stored as int FileInfo::age so we do this
+		 * check here to detect 2038 year problem
+		 * https://en.wikipedia.org/wiki/Year_2038_problem */
+		RsErr() << __PRETTY_FUNCTION__ << " period: " << period  << " > "
+		        << max_int - now << std::errc::value_too_large << std::endl;
 		return false;
-	};
+	}
 
 	if(!RsDirUtil::fileExists(path))
-		return failure("file: " + path + "not found");
+	{
+		RsErr() << __PRETTY_FUNCTION__ << " path: " << path
+		        << std::errc::no_such_file_or_directory << std::endl;
+		return false;
+	}
 
 	if(RsDirUtil::checkDirectory(path))
-		return failure("Cannot add a directory: " + path + "as extra file");
+	{
+		RsErr() << __PRETTY_FUNCTION__ << " path: " << path
+		        << std::errc::is_a_directory << std::endl;
+		return false;
+	}
 
 	FileDetails details(path, period, flags);
-	details.info.age = static_cast<int>(time(nullptr) + period);
+	details.info.age = static_cast<int>(timeOut);
 
 	{
 		RS_STACK_MUTEX(extMutex);
@@ -492,8 +484,7 @@ bool    ftExtraList::loadList(std::list<RsItem *>& load)
 
 		if (ts > (rstime_t)fi->file.age)
 		{
-			/* to old */
+			/* too old */
-			cleanupEntry(fi->file.path, TransferRequestFlags(fi->flags));
 			delete (*it);
 			continue ;
 		}

libretroshare/src/ft/ftextralist.h

@@ -60,7 +60,7 @@
 #include "pqi/p3cfgmgr.h"
 #include "util/rstime.h"
 
-class FileDetails
+class RS_DEPRECATED_FOR(FileInfo) FileDetails
 {
 	public:
 		FileDetails()
@@ -130,7 +130,11 @@ public:
 		 * file is removed after period.
 		 **/
 
-	bool 		hashExtraFile(std::string path, uint32_t period, TransferRequestFlags flags);
+	/**
+	 * Hash file, and add to the files, file is removed after period.
+	 */
+	bool hashExtraFile(
+	        std::string path, uint32_t period, TransferRequestFlags flags );
 	bool	 	hashExtraFileDone(std::string path, FileInfo &info);
 
 	/***
@@ -165,7 +169,6 @@ private:
 	/* Worker Functions */
 	void	hashAFile();
 	bool	cleanupOldFiles();
-	bool    cleanupEntry(std::string path, TransferRequestFlags flags);
 
 	mutable RsMutex extMutex;
 

libretroshare/src/ft/ftserver.cc

@@ -22,6 +22,8 @@
 
 #include <algorithm>
 #include <iostream>
+#include <limits>
+#include <system_error>
 
 #include "crypto/chacha20.h"
 //const int ftserverzone = 29539;
@@ -820,6 +822,14 @@ bool ftServer::ExtraFileRemove(const RsFileHash& hash)
 bool ftServer::ExtraFileHash(
         std::string localpath, rstime_t period, TransferRequestFlags flags )
 {
+	constexpr rstime_t uintmax = std::numeric_limits<uint32_t>::max();
+	if(period > uintmax)
+	{
+		RsErr() << __PRETTY_FUNCTION__ << " period: " << period << " > "
+		        << uintmax << std::errc::value_too_large << std::endl;
+		return false;
+	}
+
 	return mFtExtra->hashExtraFile(
 	            localpath, static_cast<uint32_t>(period), flags );
 }

libretroshare/src/retroshare/rsfiles.h

@@ -658,7 +658,8 @@ public:
 	 * @brief Get file details
 	 * @jsonapi{development}
 	 * @param[in] hash file identifier
-	 * @param[in] hintflags filtering hint (RS_FILE_HINTS_EXTRA|...|RS_FILE_HINTS_LOCAL)
+	 * @param[in] hintflags filtering hint ( RS_FILE_HINTS_UPLOAD|...|
+	 *	RS_FILE_HINTS_EXTRA|RS_FILE_HINTS_LOCAL )
 	 * @param[out] info storage for file information
 	 * @return true if file found, false otherwise
 	 */