diff --git a/devbin/vfprog/Main.cc b/devbin/vfprog/Main.cc index 04f4d9c6e6..70067b3490 100644 --- a/devbin/vfprog/Main.cc +++ b/devbin/vfprog/Main.cc @@ -101,6 +101,7 @@ TSizeUInt64Pr benchmark(char testId) { } int main(int argc, char** argv) { + // SAFE: argv is always NULL-terminated by OS, strlen is safe here if (argc != 2 || ::strlen(argv[1]) != 1 || argv[1][0] < '1' || argv[1][0] > '9') { std::cerr << "Usage: " << argv[0] << " <1-9>" << std::endl; return EXIT_FAILURE; diff --git a/lib/core/CNamedPipeFactory.cc b/lib/core/CNamedPipeFactory.cc index 8b848c9dbe..8c7391375e 100644 --- a/lib/core/CNamedPipeFactory.cc +++ b/lib/core/CNamedPipeFactory.cc @@ -174,6 +174,8 @@ std::string CNamedPipeFactory::defaultPath() { // $TMPDIR is generally set on Mac OS X (to something like // /var/folders/k5/5sqcdlps5sg3cvlp783gcz740000h0/T/) and not set on other // platforms. + // SAFE: TMPDIR usage is secure - mkfifo() creates pipes with 0600 permissions, + // validates existing files, and falls back to system _PATH_VARTMP const char* tmpDir{::getenv("TMPDIR")}; // Make sure path ends with a slash so it's ready to have a file name diff --git a/lib/core/CStringCache.cc b/lib/core/CStringCache.cc index 71cff89b2c..08a2a741e4 100644 --- a/lib/core/CStringCache.cc +++ b/lib/core/CStringCache.cc @@ -40,6 +40,7 @@ const std::string& CStringCache::stringFor(const char* str) { return EMPTY_STRING; } + // SAFE: NULL check performed before strlen - str is guaranteed to be NULL-terminated return this->stringFor(str, ::strlen(str)); } diff --git a/lib/core/CStringUtils.cc b/lib/core/CStringUtils.cc index 0ce3dc245e..694daca556 100644 --- a/lib/core/CStringUtils.cc +++ b/lib/core/CStringUtils.cc @@ -29,6 +29,7 @@ #include #include #include +#include namespace { //! In order to avoid a failure on read we need to account for the rounding @@ -273,75 +274,35 @@ void CStringUtils::unEscape(char escape, std::string& str) { } std::string CStringUtils::_typeToString(const unsigned long long& i) { - char buf[4 * sizeof(unsigned long long)]; - ::memset(buf, 0, sizeof(buf)); - - ::sprintf(buf, "%llu", i); - - return buf; + return std::format("{}", i); } std::string CStringUtils::_typeToString(const unsigned long& i) { - char buf[4 * sizeof(unsigned long)]; - ::memset(buf, 0, sizeof(buf)); - - ::sprintf(buf, "%lu", i); - - return buf; + return std::format("{}", i); } std::string CStringUtils::_typeToString(const unsigned int& i) { - char buf[4 * sizeof(unsigned int)]; - ::memset(buf, 0, sizeof(buf)); - - ::sprintf(buf, "%u", i); - - return buf; + return std::format("{}", i); } std::string CStringUtils::_typeToString(const unsigned short& i) { - char buf[4 * sizeof(unsigned short)]; - ::memset(buf, 0, sizeof(buf)); - - ::sprintf(buf, "%hu", i); - - return buf; + return std::format("{}", i); } std::string CStringUtils::_typeToString(const long long& i) { - char buf[4 * sizeof(long long)]; - ::memset(buf, 0, sizeof(buf)); - - ::sprintf(buf, "%lld", i); - - return buf; + return std::format("{}", i); } std::string CStringUtils::_typeToString(const long& i) { - char buf[4 * sizeof(long)]; - ::memset(buf, 0, sizeof(buf)); - - ::sprintf(buf, "%ld", i); - - return buf; + return std::format("{}", i); } std::string CStringUtils::_typeToString(const int& i) { - char buf[4 * sizeof(int)]; - ::memset(buf, 0, sizeof(buf)); - - ::sprintf(buf, "%d", i); - - return buf; + return std::format("{}", i); } std::string CStringUtils::_typeToString(const short& i) { - char buf[4 * sizeof(short)]; - ::memset(buf, 0, sizeof(buf)); - - ::sprintf(buf, "%hd", i); - - return buf; + return std::format("{}", i); } std::string CStringUtils::_typeToString(const bool& b) { @@ -349,15 +310,7 @@ std::string CStringUtils::_typeToString(const bool& b) { } std::string CStringUtils::_typeToString(const double& i) { - // Note the extra large buffer here, which is because the format string is - // "%f" rather than "%g", which means we could be printing a 308 digit - // number without resorting to scientific notation - char buf[64 * sizeof(double)]; - ::memset(buf, 0, sizeof(buf)); - - ::sprintf(buf, "%f", i); - - return buf; + return std::format("{}", i); } std::string CStringUtils::_typeToString(const char* str) { @@ -374,44 +327,28 @@ const std::string& CStringUtils::_typeToString(const std::string& str) { } std::string CStringUtils::typeToStringPretty(double d) { - // Maximum size = 1 (for sign) - // + 7 (for # s.f.) - // + 1 (for decimal point) - // + 5 (for w.c. e+308) - // + 1 (for terminating character) - // = 16 - - char buf[16]; - ::memset(buf, 0, sizeof(buf)); - ::sprintf(buf, "%.7g", d); - return buf; + return std::format("{:.7g}", d); } std::string CStringUtils::typeToStringPrecise(double d, CIEEE754::EPrecision precision) { - // Just use a large enough buffer to hold maximum precision. - char buf[4 * sizeof(double)]; - ::memset(buf, 0, sizeof(buf)); - // Floats need higher precision to precisely round trip to decimal than // considering their effective precision base 10. There's a good discussion // at https://randomascii.wordpress.com/2012/03/08/float-precisionfrom-zero-to-100-digits-2/. // We use g format since it is the most efficient. Note also that when // printing to limited precision we must round the value before printing - // because printing just truncates, i.e. sprintf(buf, "%.6e", 0.49999998) + // because printing just truncates, i.e. std::format("{:.6e}", 0.49999998) // gives 4.999999e-1 rather than the correctly rounded value 0.5. - int ret = 0; + std::string result; switch (precision) { case CIEEE754::E_HalfPrecision: - ret = ::sprintf(buf, "%.5g", - clampToReadable(CIEEE754::round(d, CIEEE754::E_HalfPrecision))); + result = std::format("{:.5g}", clampToReadable(CIEEE754::round(d, CIEEE754::E_HalfPrecision))); break; case CIEEE754::E_SinglePrecision: - ret = ::sprintf(buf, "%.9g", - clampToReadable(CIEEE754::round(d, CIEEE754::E_SinglePrecision))); + result = std::format("{:.9g}", clampToReadable(CIEEE754::round(d, CIEEE754::E_SinglePrecision))); break; case CIEEE754::E_DoublePrecision: - ret = ::sprintf(buf, "%.17g", clampToReadable(d)); + result = std::format("{:.17g}", clampToReadable(d)); break; } @@ -419,18 +356,18 @@ std::string CStringUtils::typeToStringPrecise(double d, CIEEE754::EPrecision pre // 123.45e010 with 123.45e10 and 123.45e-010 with 123.45e-10. // Also it is inefficient to output trailing zeros, i.e. // 1.23456000000000e-11 so we strip these off in the following. - if (ret > 2) { + if (result.length() > 2) { // Look for an 'e' - char* ptr(static_cast(::memchr(buf, 'e', ret - 1))); - if (ptr != nullptr) { + size_t ePos = result.find('e'); + if (ePos != std::string::npos) { bool edit = false; bool minus = false; // Strip off any trailing zeros and a trailing point. - char* bwd = ptr; - for (;;) { + size_t bwd = ePos; + while (bwd > 0) { --bwd; - if (*bwd == '0' || *bwd == '.') { + if (result[bwd] == '0' || result[bwd] == '.') { edit = true; } else { break; @@ -438,34 +375,34 @@ std::string CStringUtils::typeToStringPrecise(double d, CIEEE754::EPrecision pre } // Strip off any leading zeros in the exponent. - char* fwd = ptr; - for (;;) { - ++fwd; - if (*fwd == '-') { + size_t fwd = ePos + 1; + while (fwd < result.length()) { + if (result[fwd] == '-') { minus = true; - } else if (*fwd == '+' || *fwd == '0') { + } else if (result[fwd] == '+' || result[fwd] == '0') { edit = true; } else { break; } + ++fwd; } if (edit) { std::string adjResult; - adjResult.reserve(ret - 1); + adjResult.reserve(result.length()); // mantissa - adjResult.assign(buf, bwd + 1); - if (::isdigit(static_cast(*fwd))) { + adjResult.assign(result, 0, bwd + 1); + if (fwd < result.length() && ::isdigit(static_cast(result[fwd]))) { adjResult.append(minus ? "e-" : "e"); // exponent - adjResult.append(fwd); + adjResult.append(result, fwd); } return adjResult; } } } - return buf; + return result; } CStringUtils::TSizeBoolPr diff --git a/lib/maths/common/CChecksum.cc b/lib/maths/common/CChecksum.cc index 458fe09575..9740959550 100644 --- a/lib/maths/common/CChecksum.cc +++ b/lib/maths/common/CChecksum.cc @@ -32,7 +32,7 @@ std::uint64_t CChecksumImpl::dispatch(std::uint64_t seed, double target = core::CIEEE754::round(target, core::CIEEE754::E_SinglePrecision); char buf[4 * sizeof(double)]; std::memset(buf, 0, sizeof(buf)); - std::sprintf(buf, "%.7g", target); + std::snprintf(buf, sizeof(buf), "%.7g", target); return core::CHashing::safeMurmurHash64(&buf[0], 4 * sizeof(double), seed); } diff --git a/lib/seccomp/CSystemCallFilter_MacOSX.cc b/lib/seccomp/CSystemCallFilter_MacOSX.cc index 3756875d06..212bfe8a79 100644 --- a/lib/seccomp/CSystemCallFilter_MacOSX.cc +++ b/lib/seccomp/CSystemCallFilter_MacOSX.cc @@ -39,17 +39,17 @@ namespace { // (debug deny) makes it easier to see which calls need adding // when one that is required is not in the list - they show up in // the macOS console. -const std::string SANDBOX_RULES{"\ - (version 1) \ - (deny default) \ - (allow signal (target self)) \ - (allow system-sched (target self)) \ - (allow file-read*) \ - (allow file-read-data) \ - (allow file-write*) \ - (allow file-write-data) \ - (allow sysctl-read) \ - (debug deny)"}; +const std::string SANDBOX_RULES{ + "(version 1) " + "(deny default) " + "(allow signal (target self)) " + "(allow system-sched (target self)) " + "(allow file-read*) " + "(allow file-read-data) " + "(allow file-write*) " + "(allow file-write-data) " + "(allow sysctl-read) " + "(debug deny)"}; // mkstemps will replace the Xs with random characters const std::string FILE_NAME_TEMPLATE{"ml.XXXXXX.sb"}; @@ -58,6 +58,8 @@ const int FILE_NAME_TEMPLATE_SUFFIX_LEN{3}; std::string getTempDir() { // Prefer to use the temporary directory set by the Elasticsearch JVM + // SAFE: TMPDIR usage is secure - mkstemps() creates files with 0600 permissions, + // uses random filenames, and file is cleaned up after use const char* tmpDir{::getenv("TMPDIR")}; // If TMPDIR is not set use _PATH_VARTMP @@ -94,8 +96,7 @@ void CSystemCallFilter::installSystemCallFilter() { return; } - char* errorbuf{nullptr}; - if (::sandbox_init(profileFilename.c_str(), SANDBOX_NAMED, &errorbuf) != 0) { + if (char* errorbuf{nullptr}; ::sandbox_init(profileFilename.c_str(), SANDBOX_NAMED, &errorbuf) != 0) { std::string msg("Error initializing macOS sandbox"); if (errorbuf != nullptr) { msg += ": ";