ExtraFileHash check for integer overflow

When passing large periods 2038 problems was silently triggered due to
  time being stored as int in FileInfo::age, thus causing erratic
  behaviour in extra files timeout. Now period is checked and if too
  large an error is reported.
Deprecate FileDetails which is confusing dummy wrapper of FileInfo
Remove ftExtraList::cleanupEntry deadcode
This commit is contained in:
Gioacchino Mazzurco 2020-05-31 15:19:00 +02:00
parent 54364c488e
commit 295ecf368e
No known key found for this signature in database
GPG Key ID: A1FBCA3872E87051
4 changed files with 46 additions and 41 deletions

View File

@ -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)))
{
cleanupEntry(it->second.info.path, it->second.info.transfer_info_flags);
mFiles.erase(it);
}
mHashOfHash.erase(makeEncryptedHash(*rit)) ;
if (mFiles.end() != (it = mFiles.find(*rit))) 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
std::cerr << "ftExtraList::hashExtraFile() path: " << path;
std::cerr << " period: " << period;
std::cerr << " flags: " << flags;
constexpr rstime_t max_int = std::numeric_limits<int>::max();
const rstime_t now = time(nullptr);
const rstime_t timeOut = now + period;
std::cerr << std::endl;
#endif
auto failure = [](std::string errMsg)
if(timeOut > max_int)
{
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 */
cleanupEntry(fi->file.path, TransferRequestFlags(fi->flags));
/* too old */
delete (*it);
continue ;
}

View File

@ -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;

View File

@ -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 );
}

View File

@ -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
*/