[ML] Resolve SonarQube Hotspots

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;

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

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

lib/core/CStringUtils.cc

@@ -29,6 +29,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <format>
 
 namespace {
 //! In order to avoid a failure on read we need to account for the rounding
@@ -273,91 +274,43 @@ 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) {
     return (b ? "true" : "false");
 }
 
 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,98 +327,82 @@ 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;
     }
 
     // Workaround for Visual C++ 2010 misformatting - replace
     // 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<char*>(::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;
                 }
             }
 
             // 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<unsigned char>(*fwd))) {
+                adjResult.assign(result, 0, bwd + 1);
+                if (fwd < result.length() && ::isdigit(static_cast<unsigned char>(result[fwd]))) {
                     adjResult.append(minus ? "e-" : "e");
                     // exponent
-                    adjResult.append(fwd);
+                    adjResult.append(result, fwd);
                 }
                 return adjResult;
             }
         }
     }
 
-    return buf;
+    return result;
 }
 
 CStringUtils::TSizeBoolPr

lib/maths/common/CChecksum.cc

@@ -32,7 +32,7 @@ std::uint64_t CChecksumImpl<BasicChecksum>::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);
 }
 

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 += ": ";