feat(sarif): describe the rules and rate each finding's confidence

CHANGELOG.md

@@ -5,6 +5,10 @@ Notable changes are grouped by release. See [all releases](https://github.com/Co
 ## Unreleased
 
 - Added repository contribution and community documentation.
+- SARIF: `tool.driver.rules` lists each rule reported with a title and a description, and each
+  result carries its confidence (`high`, `medium` or `low`).
+- A vtable box whose only warning is `missing typeinfo`, as a valid `-fno-rtti` program logs, is
+  now a `vtable-diagnostic` warning instead of a `vtable-corrupted` error.
 
 ## v0.1.0
 

README.md

@@ -31,12 +31,15 @@ captures its output, and turns each report of the CoreTrace runtime into a findi
 | `vtable-corrupted` | CWE-843 | virtual call site |
 | `vcall-invalid-target` | CWE-843 | virtual call site |
 | `vtable-type-mismatch` | CWE-843, warning | virtual call site |
+| `vtable-diagnostic` | CWE-843, warning | virtual call site |
 
 The memory rules come from the `alloc` and `bounds` modules, which the default module set
 enables. The vtable rules come from the `vtable` module's diagnostics
 (`--ct-modules=alloc,vtable --ct-vtable-diag`): each box the runtime logs at `WARN` level is one
-finding, its message listing the box's warnings and its rule the most specific one. Boxes logged
-at `INFO` level are tracing and produce no finding.
+finding, its message listing the box's warnings and its rule the most specific one. A box with
+no more specific warning is `vtable-diagnostic`: `missing typeinfo` alone, which a valid program
+built with `-fno-rtti` produces, is one. Boxes logged at `INFO` level are tracing and produce no
+finding.
 
 Sites carry the path the source was compiled from, so run the analyzer from the directory the
 static analyzers report paths from.
@@ -125,9 +128,13 @@ $ echo $?
 ```
 
 `--format sarif` prints a SARIF 2.1.0 log instead of the text summary: one result per finding
-with `ruleId`, `level`, `message.text` and `properties.cwe`, the faulting access as
-`locations[0]` and the allocation site as `relatedLocations[0]` (`id` 0, "allocated here"). In
-`--test-dir` mode, the log holds the findings of every program in one run.
+with `ruleId`, `level`, `message.text`, `properties.cwe` and `properties.confidence`, the faulting
+access as `locations[0]` and the allocation site as `relatedLocations[0]` (`id` 0, "allocated
+here"). The confidence is `high` for a fault observed while the program ran, `medium` for a vtable
+type mismatch, which may be a legitimate cast, and `low` for a vtable report without a more
+specific rule. `tool.driver.rules` lists each rule reported, with a title (`shortDescription`) and
+what it detects (`fullDescription`), for a consumer such as GitHub code scanning to explain the
+finding. In `--test-dir` mode, the log holds the findings of every program in one run.
 
 ```console
 $ runtime-analyzer --format sarif -- --ct-modules=alloc,bounds findings/heap_overflow_write.c
@@ -141,12 +148,14 @@ $ runtime-analyzer --format sarif -- --ct-modules=alloc,bounds findings/heap_ove
           "locations": [ { "physicalLocation": { "artifactLocation": { "uri": "findings/heap_overflow_write.c" },
                                                  "region": { "startColumn": 15, "startLine": 7 } } } ],
           "message": { "text": "heap-buffer-overflow WRITE of size 4" },
-          "properties": { "cwe": "CWE-122" },
+          "properties": { "confidence": "high", "cwe": "CWE-122" },
           "relatedLocations": [ { "id": 0, "message": { "text": "allocated here" }, ... } ],
           "ruleId": "heap-buffer-overflow"
         }
       ],
-      "tool": { "driver": { "name": "coretrace-runtime-analyzer", ... } }
+      "tool": { "driver": { "name": "coretrace-runtime-analyzer",
+                            "rules": [ { "id": "heap-buffer-overflow",
+                                         "shortDescription": { "text": "Heap buffer overflow" }, ... } ], ... } }
     }
   ],
   "version": "2.1.0"
@@ -230,7 +239,7 @@ ctest --test-dir build --output-on-failure
 | `findings_unit`, `process_unit` | the report parser and the process runner (`unittests/`) |
 | `cli_args_unit` | argument parsing, every option and every invalid spelling, the compiler's `-o`, the compile-only and `-x` flags, the C-as-C++ and `DEBUG` heuristics, the compiler arguments of a run, the artifact names of a batch (`src/cli_args.hpp`) |
 | `events_unit` | the CoreTrace event lines of a run and their summary, ANSI colours stripped, and the verdict drawn from a result (`src/events.hpp`) |
-| `sarif_unit` | the SARIF writer against the exact log a consumer reads: empty, an error with its locations, a warning without one |
+| `sarif_unit` | the SARIF writer against the exact log a consumer reads: empty, an error with its locations, a warning without one; every rule listed with a title and a description, and each finding's confidence |
 | `api_unit` | the library API: `Run` and `RunBatch` on real programs, a heap overflow, a compile-only run, a hang against the timeout, a batch, the refused and empty cases |
 | `consumer` | a project fetching this repository the way ctrace does (`unittests/consumer/`) configures, builds, links `coretrace::runtime-analyzer_lib` and analyzes a program |
 | `fixtures` | every source under `test/` states what it proves in its header (`// args:`, `// expect:`), and the analyzer proves it: findings with rule, CWE, level, faulting line and allocation site, or none; the program's exit status and output when stated. A fixture without an expectation fails (`test/check_fixtures.py`) |

src/findings.cpp

@@ -102,10 +102,11 @@ namespace coretrace::runtime_analyzer
             {"vptr on freed object", "vtable-use-after-free", "CWE-416", Severity::Error},
             {"vtable resolve failed", "vtable-corrupted", "CWE-843", Severity::Error},
             {"no vptr", "vtable-corrupted", "CWE-843", Severity::Error},
-            {"missing typeinfo", "vtable-corrupted", "CWE-843", Severity::Error},
             {"target in non-exec memory", "vcall-invalid-target", "CWE-843", Severity::Error},
             {"static!=dynamic type", "vtable-type-mismatch", "CWE-843", Severity::Warning},
             {"module mismatch: ", "vtable-type-mismatch", "CWE-843", Severity::Warning},
+            // Valid under -fno-rtti, so it never outranks another warning of the box.
+            {"missing typeinfo", "vtable-diagnostic", "CWE-843", Severity::Warning},
         };
 
         // A box the runtime logged at WARN level: "[VTABLE]" or "[VCALL]", then "│ label : value │"
@@ -251,6 +252,72 @@ namespace coretrace::runtime_analyzer
         }
     } // namespace
 
+    std::optional<RuleDescription> DescribeRule(std::string_view rule)
+    {
+        struct DescribedRule
+        {
+            std::string_view rule;
+            RuleDescription description;
+        };
+        // Every rule ParseFindings reports: the bounds rules, double-free and memory-leak, and the
+        // vtable rules with their fallback.
+        static constexpr DescribedRule kRules[] = {
+            {"heap-buffer-overflow",
+             {"Heap buffer overflow",
+              "A read or a write past the bounds of a heap allocation, "
+              "observed while the program ran.",
+              "high"}},
+            {"stack-buffer-overflow",
+             {"Stack buffer overflow",
+              "A read or a write past the bounds of a stack object, "
+              "observed while the program ran.",
+              "high"}},
+            {"heap-use-after-free",
+             {"Use of freed heap memory",
+              "A read or a write to heap memory after it was freed, "
+              "observed while the program ran.",
+              "high"}},
+            {"double-free",
+             {"Double free", "Heap memory freed a second time, observed while the program ran.",
+              "high"}},
+            {"memory-leak",
+             {"Memory leak", "Heap memory the program allocated and had not freed when it ended.",
+              "high"}},
+            {"vtable-null-this",
+             {"Virtual call through a null object",
+              "A virtual call made through a null pointer to an object.", "high"}},
+            {"vtable-use-after-free",
+             {"Virtual call through a freed object",
+              "A virtual call made through an object whose memory was already freed.", "high"}},
+            {"vtable-corrupted",
+             {"Virtual call through a corrupted vtable",
+              "A virtual call made through an object whose vtable pointer is missing or "
+              "invalid.",
+              "high"}},
+            {"vcall-invalid-target",
+             {"Virtual call to non-executable memory",
+              "A virtual call whose target address is not in executable memory.", "high"}},
+            {"vtable-type-mismatch",
+             {"Object of an unexpected dynamic type",
+              "At a virtual call, the object's dynamic type differs from its static type, or "
+              "comes from another module: a possible type confusion, or a legitimate cast.",
+              "medium"}},
+            {"vtable-diagnostic",
+             {"Other virtual call issue",
+              "A vtable or virtual call problem the runtime reported without a more specific "
+              "rule, such as a vtable without type information, as built with -fno-rtti.",
+              "low"}},
+        };
+        for (const DescribedRule& described : kRules)
+        {
+            if (described.rule == rule)
+            {
+                return described.description;
+            }
+        }
+        return std::nullopt;
+    }
+
     std::vector<Finding> ParseFindings(std::string_view output)
     {
         std::vector<Finding> findings;

src/findings.hpp

@@ -4,13 +4,26 @@
 
 #include "runtime_analyzer.hpp"
 
+#include <optional>
 #include <string_view>
 #include <vector>
 
 namespace coretrace::runtime_analyzer
 {
     // The memory errors the CoreTrace runtime reported in `output`, without ANSI escapes.
     [[nodiscard]] std::vector<Finding> ParseFindings(std::string_view output);
+
+    // What a rule checks, for a SARIF consumer to explain the rule's findings, and how sure a
+    // finding of it is: "high" for a fault observed while the program ran, lower for an inference.
+    struct RuleDescription
+    {
+        std::string_view title;
+        std::string_view description;
+        std::string_view confidence;
+    };
+
+    // The description of `rule`; nothing for a rule this analyzer does not report.
+    [[nodiscard]] std::optional<RuleDescription> DescribeRule(std::string_view rule);
 } // namespace coretrace::runtime_analyzer
 
 #endif // CORETRACE_RUNTIME_ANALYZER_FINDINGS_HPP

src/sarif.cpp

@@ -1,10 +1,14 @@
 // SPDX-License-Identifier: Apache-2.0
 #include "sarif.hpp"
 
+#include "findings.hpp"
+
 #include <llvm/Support/FormatVariadic.h>
 #include <llvm/Support/JSON.h>
 #include <llvm/Support/raw_os_ostream.h>
 
+#include <set>
+#include <string>
 #include <utility>
 
 namespace coretrace::runtime_analyzer
@@ -27,13 +31,32 @@ namespace coretrace::runtime_analyzer
             };
         }
 
+        // A rule as the log lists it: its id, and its title and description when it has them.
+        [[nodiscard]] llvm::json::Object Rule(const std::string& rule)
+        {
+            llvm::json::Object listed{{"id", rule}};
+            if (const std::optional<RuleDescription> described = DescribeRule(rule))
+            {
+                listed["shortDescription"] =
+                    llvm::json::Object{{"text", std::string(described->title)}};
+                listed["fullDescription"] =
+                    llvm::json::Object{{"text", std::string(described->description)}};
+            }
+            return listed;
+        }
+
         [[nodiscard]] llvm::json::Object Result(const Finding& finding)
         {
+            llvm::json::Object properties{{"cwe", finding.cwe}};
+            if (const std::optional<RuleDescription> described = DescribeRule(finding.rule))
+            {
+                properties["confidence"] = std::string(described->confidence);
+            }
             llvm::json::Object result{
                 {"ruleId", finding.rule},
                 {"level", finding.severity == Severity::Error ? "error" : "warning"},
                 {"message", llvm::json::Object{{"text", finding.message}}},
-                {"properties", llvm::json::Object{{"cwe", finding.cwe}}},
+                {"properties", std::move(properties)},
             };
             if (finding.location)
             {
@@ -53,14 +76,21 @@ namespace coretrace::runtime_analyzer
     void WriteSarif(std::ostream& out, const std::vector<Finding>& findings)
     {
         llvm::json::Array results;
+        llvm::json::Array rules;
+        std::set<std::string> listed;
         for (const Finding& finding : findings)
         {
             results.push_back(Result(finding));
+            if (listed.insert(finding.rule).second)
+            {
+                rules.push_back(Rule(finding.rule));
+            }
         }
 
         llvm::json::Object driver{
             {"name", "coretrace-runtime-analyzer"},
             {"informationUri", "https://github.com/CoreTrace/coretrace-runtime-analyzer"},
+            {"rules", std::move(rules)},
         };
         llvm::json::Object run{
             {"tool", llvm::json::Object{{"driver", std::move(driver)}}},

test/ct_vtable_diag_fake.cpp

@@ -22,7 +22,7 @@ int main()
     }
 
     table[0] = nullptr; // offset-to-top
-    table[1] = nullptr; // missing typeinfo
+    table[1] = nullptr; // missing typeinfo, outranked by "vtable resolve failed"
     table[2] = nullptr; // fake entry
 
     FakeObject obj{};

test/ct_vtable_no_rtti.cpp

@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: Apache-2.0
+// args: --ct-modules=alloc,vtable --ct-vtable-diag -fno-rtti
+// expect: rule=vtable-diagnostic cwe=CWE-843 level=warning line=24
+#include <cstdio>
+
+struct Base
+{
+    virtual int Value() const
+    {
+        return 1;
+    }
+};
+
+struct Derived : Base
+{
+    int Value() const override
+    {
+        return 2;
+    }
+};
+
+[[gnu::noinline]] int Call(const Base& base)
+{
+    return base.Value();
+}
+
+int main()
+{
+    Derived derived;
+    std::printf("%d\n", Call(derived));
+    return 0;
+}

unittests/findings_test.cpp

@@ -265,6 +265,7 @@ namespace
         };
         constexpr Case cases[] = {
             {"vptr on freed object", "vtable-use-after-free", "CWE-416", Severity::Error},
+            {"missing typeinfo", "vtable-diagnostic", "CWE-843", Severity::Warning},
             {"missing typeinfo|vtable resolve failed", "vtable-corrupted", "CWE-843",
              Severity::Error},
             {"no vptr", "vtable-corrupted", "CWE-843", Severity::Error},
@@ -276,6 +277,8 @@ namespace
             {"no vptr|vptr on freed object", "vtable-use-after-free", "CWE-416", Severity::Error},
             {"static!=dynamic type|target in non-exec memory", "vcall-invalid-target", "CWE-843",
              Severity::Error},
+            {"missing typeinfo|target in non-exec memory", "vcall-invalid-target", "CWE-843",
+             Severity::Error},
         };
         for (const Case& c : cases)
         {