Security scan issues for v3.5.1

[sohankunkerkar]

Security scan issues for v3.5.1

[sohankunkerkar]

I came up with this patch. Let me know if this makes sense:

From e3415c1d13232c4ad1286931c1151fc6ae2366d8 Mon Sep 17 00:00:00 2001
From: Sohan Kunkerkar <[email protected]>
Date: Tue, 9 Jan 2024 16:04:19 -0500
Subject: [PATCH] Fix rhel-9 SAST scan issues

---
 src/catch2/catch_test_case_info.cpp                     | 6 +++++-
 src/catch2/internal/catch_random_seed_generation.cpp    | 6 +++---
 src/catch2/internal/catch_random_seed_generation.hpp    | 2 +-
 src/catch2/reporters/catch_reporter_cumulative_base.cpp | 2 +-
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/catch2/catch_test_case_info.cpp b/src/catch2/catch_test_case_info.cpp
index c38ee55a..9cd76be9 100644
--- a/src/catch2/catch_test_case_info.cpp
+++ b/src/catch2/catch_test_case_info.cpp
@@ -88,8 +88,12 @@ namespace Catch {
             --lastDot;
 
             size_t nameStart = lastDot;
-            while (nameStart > 0 && filename[nameStart - 1] != '/' && filename[nameStart - 1] != '\\') {
+            if (nameStart < filename.size()) {
+               while (nameStart > 0 && filename[nameStart - 1] != '/' && filename[nameStart - 1] != '\\') {
                 --nameStart;
+                }
+            } else {
+                nameStart = 0;
             }
 
             return filename.substr(nameStart, lastDot - nameStart);
diff --git a/src/catch2/internal/catch_random_seed_generation.cpp b/src/catch2/internal/catch_random_seed_generation.cpp
index fdc3fa19..6ae880aa 100644
--- a/src/catch2/internal/catch_random_seed_generation.cpp
+++ b/src/catch2/internal/catch_random_seed_generation.cpp
@@ -16,15 +16,15 @@
 
 namespace Catch {
 
-    std::uint32_t generateRandomSeed( GenerateFrom from ) {
+    std::uint64_t generateRandomSeed( GenerateFrom from ) {
         switch ( from ) {
         case GenerateFrom::Time:
-            return static_cast<std::uint32_t>( std::time( nullptr ) );
+            return static_cast<std::uint64_t>( std::time( nullptr ) );
 
         case GenerateFrom::Default:
         case GenerateFrom::RandomDevice: {
             std::random_device rd;
-            return Detail::fillBitsFrom<std::uint32_t>( rd );
+            return Detail::fillBitsFrom<std::uint64_t>( rd );
         }
 
         default:
diff --git a/src/catch2/internal/catch_random_seed_generation.hpp b/src/catch2/internal/catch_random_seed_generation.hpp
index d0d6fb24..3c4e03d6 100644
--- a/src/catch2/internal/catch_random_seed_generation.hpp
+++ b/src/catch2/internal/catch_random_seed_generation.hpp
@@ -19,7 +19,7 @@ namespace Catch {
         Default
     };
 
-    std::uint32_t generateRandomSeed(GenerateFrom from);
+    std::uint64_t generateRandomSeed(GenerateFrom from);
 
 } // end namespace Catch
 
diff --git a/src/catch2/reporters/catch_reporter_cumulative_base.cpp b/src/catch2/reporters/catch_reporter_cumulative_base.cpp
index 5e106326..e43c3a23 100644
--- a/src/catch2/reporters/catch_reporter_cumulative_base.cpp
+++ b/src/catch2/reporters/catch_reporter_cumulative_base.cpp
@@ -87,8 +87,8 @@ namespace Catch {
             if ( it == parentNode.childSections.end() ) {
                 auto newNode =
                     Detail::make_unique<SectionNode>( incompleteStats );
-                node = newNode.get();
                 parentNode.childSections.push_back( CATCH_MOVE( newNode ) );
+                node = newNode.get();
             } else {
                 node = it->get();
             }
-- 
2.41.0

[sohankunkerkar]

cc @horenmar

[horenmar]

In reverse order:

• The proposed changes for reporter_cumulative_base.cpp might make the warning go away, but they also completely break the section trackers. node exists to paper over getting a non-owning pointer to an object that is owned by parent tracker. There is no UAF there and both warnings from your tool are wrong (and honestly somewhat contradictory. Am I using an internal representation of the object, or am I using a freed pointer? These are obviously different cases)
• The rest of the code uses 32 bit rng seeds, there is no reason to generate more bits to toss them away later.
• Overflowing the search for . in a filename might actually cause an issue and I'll make a note to fix it. I do not consider this priority, because it requires combination of a cpp file without any extension in the name, and -# flag.