From 480296911f442e16d3f81be8d1adf2902e07d734 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Fri, 15 Apr 2022 13:26:25 +0100 Subject: Safe numeric conversions --- netfs/fuse/fuseApp.cpp | 5 +- netfs/fuse/fuseApp.h | 4 +- netfs/fuse/fuseDirs.cpp | 3 +- netfs/fuse/fuseFiles.cpp | 17 +++---- netfs/fuse/fuseFiles.h | 2 +- netfs/fuse/fuseMisc.cpp | 9 ++-- netfs/ice/numeric.cpp | 7 +++ netfs/ice/numeric.h | 51 ++++++++++++++++++++ netfs/ice/typeConverter.cpp | 109 +++++++++++++++++++------------------------ netfs/unittests/testCore.cpp | 3 +- 10 files changed, 129 insertions(+), 81 deletions(-) create mode 100644 netfs/ice/numeric.cpp create mode 100644 netfs/ice/numeric.h diff --git a/netfs/fuse/fuseApp.cpp b/netfs/fuse/fuseApp.cpp index b6d59b1..cfd9689 100644 --- a/netfs/fuse/fuseApp.cpp +++ b/netfs/fuse/fuseApp.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -19,7 +20,7 @@ namespace AdHoc { template class CallCacheable; } -NetFS::FuseApp::FuseApp(Ice::StringSeq && a) : iceArgs(std::move(a)), sessionOpened(false), openHandleId(0) { } +NetFS::FuseApp::FuseApp(Ice::StringSeq && a) : iceArgs(std::move(a)) { } NetFS::FuseApp::~FuseApp() { @@ -296,6 +297,6 @@ NetFS::ReqEnv NetFS::FuseApp::reqEnv() { struct fuse_context * c = fuse_get_context(); - const auto t = converter.mapper->mapFileSystem(c->uid, c->gid); + const auto t = converter.mapper->mapFileSystem(safe {c->uid}, safe {c->gid}); return {t.username, t.groupname}; } diff --git a/netfs/fuse/fuseApp.h b/netfs/fuse/fuseApp.h index 9eafb7f..187e2ba 100644 --- a/netfs/fuse/fuseApp.h +++ b/netfs/fuse/fuseApp.h @@ -103,13 +103,13 @@ namespace NetFS { NetFS::VolumePrxPtr volume; NetFS::ServicePrxPtr service; Glacier2::SessionPrxPtr session; - bool sessionOpened; + bool sessionOpened {false}; std::string mountPoint; OpenDirs openDirs; OpenFiles openFiles; - int openHandleId; + int openHandleId {0}; EntryTypeConverter converter; diff --git a/netfs/fuse/fuseDirs.cpp b/netfs/fuse/fuseDirs.cpp index b03282e..bc13cd1 100644 --- a/netfs/fuse/fuseDirs.cpp +++ b/netfs/fuse/fuseDirs.cpp @@ -1,6 +1,7 @@ #include "fuseDirs.h" #include #include +#include namespace NetFS { FuseApp::OpenDir::OpenDir(DirectoryPrxPtr r, std::string p) : remote(std::move(r)), path(std::move(p)) { } @@ -75,7 +76,7 @@ namespace NetFS { FuseApp::mkdir(const char * p, mode_t m) { try { - volume->mkdir(reqEnv(), p, m); + volume->mkdir(reqEnv(), p, safe {m}); return 0; } catch (SystemError & e) { diff --git a/netfs/fuse/fuseFiles.cpp b/netfs/fuse/fuseFiles.cpp index 8014732..3d23338 100644 --- a/netfs/fuse/fuseFiles.cpp +++ b/netfs/fuse/fuseFiles.cpp @@ -4,6 +4,7 @@ #include #include #include +#include namespace NetFS { FuseApp::OpenFile::WriteState::WriteState() : future(promise.get_future().share()) { } @@ -67,7 +68,7 @@ namespace NetFS { FuseApp::create(const char * p, mode_t m, struct fuse_file_info * fi) { try { - auto remote = volume->create(reqEnv(), p, fi->flags, m); + auto remote = volume->create(reqEnv(), p, fi->flags, safe {m}); setProxy(fi->fh, remote, p, fi->flags); return 0; } @@ -143,20 +144,20 @@ namespace NetFS { FuseApp::read(const char *, char * buf, size_t s, off_t o, struct fuse_file_info * fi) { try { - auto cpy = [buf](const auto && data) { + auto cpy = [buf](const auto && data) -> int { memcpy(buf, &data.front(), data.size()); - return data.size(); + return safe {data.size()}; }; auto of = getProxy(fi->fh); auto remote = of->remote; if (fcr->Async) { auto p = waitOnWriteRangeAndThen>(s, o, of, [o, s, &remote](const auto &) { - return remote->readAsync(o, s); + return remote->readAsync(o, safe {s}); }); return cpy(p.get()); } else { - return cpy(remote->read(o, s)); + return cpy(remote->read(o, safe {s})); } } catch (SystemError & e) { @@ -191,9 +192,9 @@ namespace NetFS { }); } else { - remote->write(o, s, Buffer(buf, buf + s)); + remote->write(o, safe {s}, Buffer(buf, buf + s)); } - return s; + return safe {s}; } catch (SystemError & e) { return -e.syserrno; @@ -211,7 +212,7 @@ namespace NetFS { auto remote_out = of_out->remote; of_in->wait(); of_out->wait(); - return remote_in->copyrange(remote_out, offset_in, offset_out, size, flags); + return remote_in->copyrange(remote_out, offset_in, offset_out, safe {size}, flags); } catch (SystemError & e) { return -e.syserrno; diff --git a/netfs/fuse/fuseFiles.h b/netfs/fuse/fuseFiles.h index 9143836..16ded5e 100644 --- a/netfs/fuse/fuseFiles.h +++ b/netfs/fuse/fuseFiles.h @@ -22,7 +22,7 @@ namespace NetFS { std::promise promise; std::shared_future future; }; - using BGs = boost::icl::interval_map>; + using BGs = boost::icl::interval_map>; static BGs::interval_type range(off_t, size_t); BGs bg; mutable std::shared_mutex _lock; diff --git a/netfs/fuse/fuseMisc.cpp b/netfs/fuse/fuseMisc.cpp index 36a695b..8069234 100644 --- a/netfs/fuse/fuseMisc.cpp +++ b/netfs/fuse/fuseMisc.cpp @@ -1,6 +1,7 @@ #include "fuseApp.h" #include #include +#include int NetFS::FuseApp::access(const char * p, int a) @@ -12,7 +13,7 @@ int NetFS::FuseApp::chmod(const char * p, mode_t m, fuse_file_info *) { try { - volume->chmod(reqEnv(), p, m); + volume->chmod(reqEnv(), p, safe {m}); return 0; } catch (NetFS::SystemError & e) { @@ -24,7 +25,7 @@ int NetFS::FuseApp::chown(const char * p, uid_t u, gid_t g, fuse_file_info *) { try { - volume->chown(reqEnv(), p, u, g); + volume->chown(reqEnv(), p, safe {u}, safe {g}); return 0; } catch (NetFS::SystemError & e) { @@ -48,7 +49,7 @@ int NetFS::FuseApp::mknod(const char * p, mode_t mode, dev_t dev) { try { - volume->mknod(reqEnv(), p, mode, dev); + volume->mknod(reqEnv(), p, safe {mode}, safe {dev}); return 0; } catch (NetFS::SystemError & e) { @@ -86,7 +87,7 @@ int NetFS::FuseApp::rename(const char * p1, const char * p2, unsigned int flags) { try { - volume->rename(reqEnv(), p1, p2, flags); + volume->rename(reqEnv(), p1, p2, safe {flags}); return 0; } catch (NetFS::SystemError & e) { diff --git a/netfs/ice/numeric.cpp b/netfs/ice/numeric.cpp new file mode 100644 index 0000000..2ec05af --- /dev/null +++ b/netfs/ice/numeric.cpp @@ -0,0 +1,7 @@ +#include "numeric.h" + +void +safe_base::out_of_range(const char * const function) +{ + throw std::out_of_range {function}; +} diff --git a/netfs/ice/numeric.h b/netfs/ice/numeric.h new file mode 100644 index 0000000..3b49f4b --- /dev/null +++ b/netfs/ice/numeric.h @@ -0,0 +1,51 @@ +#ifndef NUMERIC_H +#define NUMERIC_H + +#include +#include +#include +#include +#include +#include +#include + +struct DLL_PUBLIC safe_base { +protected: + [[noreturn]] static void out_of_range(const char * const); +}; + +template struct safe : safe_base { + constexpr explicit safe(T v) noexcept : value {v} { } + + // NOLINTNEXTLINE(hicpp-explicit-conversions) + template constexpr inline operator R() const + { + if constexpr (std::cmp_less_equal(std::numeric_limits::min(), min) + && std::cmp_greater_equal(std::numeric_limits::max(), max)) { + return value; + } + else { + if (!std::in_range(value)) { + [[unlikely]] out_of_range(__PRETTY_FUNCTION__); + } + return static_cast(value); + } + } + + template constexpr inline operator std::optional() const + { + return this->operator R(); + } + + template constexpr inline operator Ice::optional() const + { + return this->operator R(); + } + +private: + T value; + constexpr static T min {std::numeric_limits::min()}; + constexpr static T max {std::numeric_limits::max()}; +}; + +#endif diff --git a/netfs/ice/typeConverter.cpp b/netfs/ice/typeConverter.cpp index 2d9914a..a0c8437 100644 --- a/netfs/ice/typeConverter.cpp +++ b/netfs/ice/typeConverter.cpp @@ -1,42 +1,29 @@ #include "typeConverter.h" -#include +#include "numeric.h" EntryTypeConverter::EntryTypeConverter(NetFS::Mapping::MapperPtr m) : mapper(std::move(m)) { } -// Wrapper function to assure no unexpected corruption occurs -// during narrowing assignments. -// (bugprone-narrowing-conversions) -template -inline void -safeAssign(T & t, const S & s) -{ - t = boost::numeric_cast(s); -} - struct stat EntryTypeConverter::convert(const NetFS::Attr & a) const { auto map = mapper->mapTransport(a.uid, a.gid); struct stat s { }; - s.st_dev = a.dev; - s.st_ino = a.inode; - s.st_mode = a.mode & ~map.mask; - s.st_nlink = a.links; + s.st_dev = safe {a.dev}; + s.st_ino = safe {a.inode}; + s.st_mode = safe {a.mode & ~map.mask}; + s.st_nlink = safe {a.links}; s.st_uid = map.uid; s.st_gid = map.gid; - s.st_rdev = a.rdev; - s.st_size = a.size; - s.st_blksize = a.blockSize; - s.st_blocks = a.blocks; - s.st_atime = a.atime; - s.st_atim.tv_sec = a.atime; - s.st_atim.tv_nsec = 0; - s.st_mtime = a.mtime; - s.st_mtim.tv_sec = a.mtime; - s.st_mtim.tv_nsec = 0; - s.st_ctime = a.ctime; - s.st_ctim.tv_sec = a.ctime; - s.st_ctim.tv_nsec = 0; + s.st_rdev = safe {a.rdev}; + s.st_size = safe {a.size}; + s.st_blksize = safe {a.blockSize}; + s.st_blocks = safe {a.blocks}; + s.st_atime = safe {a.atime}; + s.st_atim.tv_sec = safe {a.atime}; + s.st_mtime = safe {a.mtime}; + s.st_mtim.tv_sec = safe {a.mtime}; + s.st_ctime = safe {a.ctime}; + s.st_ctim.tv_sec = safe {a.ctime}; return s; } @@ -44,38 +31,37 @@ struct statvfs TypeConverter::convert(const NetFS::VFS & v) const { struct statvfs vfs { }; - vfs.f_bsize = v.blockSize; - vfs.f_frsize = v.fragmentSize; - vfs.f_blocks = v.blocks; - vfs.f_bfree = v.freeBlocks; - vfs.f_bavail = v.availBlocks; - vfs.f_files = v.files; - vfs.f_ffree = v.freeFiles; - vfs.f_favail = v.availFiles; - vfs.f_fsid = 0; - vfs.f_flag = v.flags; - vfs.f_namemax = v.maxNameLen; + vfs.f_bsize = safe {v.blockSize}; + vfs.f_frsize = safe {v.fragmentSize}; + vfs.f_blocks = safe {v.blocks}; + vfs.f_bfree = safe {v.freeBlocks}; + vfs.f_bavail = safe {v.availBlocks}; + vfs.f_files = safe {v.files}; + vfs.f_ffree = safe {v.freeFiles}; + vfs.f_favail = safe {v.availFiles}; + vfs.f_flag = safe {v.flags}; + vfs.f_namemax = safe {v.maxNameLen}; return vfs; } NetFS::Attr EntryTypeConverter::convert(const struct stat & s) const { - auto map = mapper->mapFileSystem(s.st_uid, s.st_gid); + auto map = mapper->mapFileSystem(safe {s.st_uid}, safe {s.st_gid}); NetFS::Attr a {}; - a.dev = s.st_dev; - a.inode = s.st_ino; - safeAssign(a.mode, s.st_mode & ~map.mask); - a.links = s.st_nlink; + a.dev = safe {s.st_dev}; + a.inode = safe {s.st_ino}; + a.mode = safe {s.st_mode & ~map.mask}; + a.links = safe {s.st_nlink}; a.uid = std::move(map.username); a.gid = std::move(map.groupname); - a.rdev = s.st_rdev; - a.size = s.st_size; - a.blockSize = s.st_blksize; - a.blocks = s.st_blocks; - a.atime = s.st_atime; - a.mtime = s.st_mtime; - a.ctime = s.st_ctime; + a.rdev = safe {s.st_rdev}; + a.size = safe {s.st_size}; + a.blockSize = safe {s.st_blksize}; + a.blocks = safe {s.st_blocks}; + a.atime = safe {s.st_atime}; + a.mtime = safe {s.st_mtime}; + a.ctime = safe {s.st_ctime}; return a; } @@ -83,16 +69,15 @@ NetFS::VFS TypeConverter::convert(const struct statvfs & s) const { NetFS::VFS t {}; - safeAssign(t.blockSize, s.f_bsize); - safeAssign(t.fragmentSize, s.f_frsize); - t.blocks = s.f_blocks; - t.freeBlocks = s.f_bfree; - t.availBlocks = s.f_bavail; - t.files = s.f_files; - t.freeFiles = s.f_ffree; - t.availFiles = s.f_favail; - t.FSID = 0; - safeAssign(t.flags, s.f_flag); - safeAssign(t.maxNameLen, s.f_namemax); + t.blockSize = safe {s.f_bsize}; + t.fragmentSize = safe {s.f_frsize}; + t.blocks = safe {s.f_blocks}; + t.freeBlocks = safe {s.f_bfree}; + t.availBlocks = safe {s.f_bavail}; + t.files = safe {s.f_files}; + t.freeFiles = safe {s.f_ffree}; + t.availFiles = safe {s.f_favail}; + t.flags = safe {s.f_flag}; + t.maxNameLen = safe {s.f_namemax}; return t; } diff --git a/netfs/unittests/testCore.cpp b/netfs/unittests/testCore.cpp index bb22858..c99999f 100644 --- a/netfs/unittests/testCore.cpp +++ b/netfs/unittests/testCore.cpp @@ -487,7 +487,8 @@ BOOST_AUTO_TEST_CASE(chown) BOOST_REQUIRE_EQUAL(fuse->getattr("/dir", &st, nullptr), 0); BOOST_REQUIRE_EQUAL(st.st_uid, getuid()); BOOST_REQUIRE_EQUAL(st.st_gid, getgid()); - BOOST_REQUIRE_EQUAL(fuse->chown("/dir", -2, getgid(), nullptr), -EPERM); + BOOST_REQUIRE_EQUAL(fuse->chown("/dir", -2, getgid(), nullptr), -ENOSYS); + BOOST_REQUIRE_EQUAL(fuse->chown("/dir", 99999999, getgid(), nullptr), -EPERM); BOOST_REQUIRE_EQUAL(fuse->rmdir("/dir"), 0); } -- cgit v1.2.3