From f5fedb62781d6d0f34e89ba5ead040a3456fa5cd Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 04:58:59 +0200 Subject: [PATCH 1/9] test(sarif): every rule must be listed with a title and a description The SARIF log has no tool.driver.rules: a consumer such as GitHub code scanning or CoreTrace's merged log can only show the bare rule id. Each of the eleven rules the analyzer reports must be listed once, with a title other than its id and a description. Refs #29 --- unittests/sarif_test.cpp | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/unittests/sarif_test.cpp b/unittests/sarif_test.cpp index 4a27b40..d835638 100644 --- a/unittests/sarif_test.cpp +++ b/unittests/sarif_test.cpp @@ -4,6 +4,8 @@ #include "expect.hpp" +#include + #include #include #include @@ -141,6 +143,59 @@ namespace Expect(Sarif({finding}) == expected, "one warning: no locations, no column when the runtime gave none"); } + + // The one run of the log Sarif() writes for `findings`. + [[nodiscard]] llvm::json::Object Run(const std::vector& findings) + { + llvm::Expected log = llvm::json::parse(Sarif(findings)); + if (!log) + { + llvm::consumeError(log.takeError()); + return {}; + } + const llvm::json::Array* runs = log->getAsObject()->getArray("runs"); + return runs != nullptr && !runs->empty() ? *(*runs)[0].getAsObject() : llvm::json::Object{}; + } + + [[nodiscard]] Finding OfRule(std::string rule) + { + return Finding{std::move(rule), "CWE-0", Severity::Error, "m", std::nullopt, std::nullopt}; + } + + // A SARIF consumer (GitHub code scanning, an IDE, CoreTrace's merged log) explains a finding + // from its rule: every rule the analyzer reports is listed once, with a title and a + // description. + void EveryRuleIsDescribed() + { + const std::vector rules = { + "heap-buffer-overflow", "stack-buffer-overflow", "heap-use-after-free", + "double-free", "memory-leak", "vtable-null-this", + "vtable-use-after-free", "vtable-corrupted", "vcall-invalid-target", + "vtable-type-mismatch", "vtable-diagnostic"}; + for (const std::string& rule : rules) + { + llvm::json::Object run = Run({OfRule(rule), OfRule(rule)}); + const llvm::json::Object* driver = run.getObject("tool") != nullptr + ? run.getObject("tool")->getObject("driver") + : nullptr; + const llvm::json::Array* listed = + driver != nullptr ? driver->getArray("rules") : nullptr; + const llvm::json::Object* entry = + listed != nullptr && listed->size() == 1 ? (*listed)[0].getAsObject() : nullptr; + const auto text = [&](llvm::StringRef key) -> llvm::StringRef + { + const llvm::json::Object* message = + entry != nullptr ? entry->getObject(key) : nullptr; + const std::optional value = + message != nullptr ? message->getString("text") : std::nullopt; + return value.value_or(""); + }; + Expect(entry != nullptr && entry->getString("id") == rule && + !text("shortDescription").empty() && !text("fullDescription").empty() && + text("shortDescription") != rule, + rule + ": listed once, with a title and a description"); + } + } } // namespace int main() @@ -148,5 +203,6 @@ int main() EmptyLog(); FindingWithLocations(); WarningWithoutLocation(); + EveryRuleIsDescribed(); return unittests::Finish("sarif_test"); } From 9bcbb5c4307228160042fce492d5237b3b7aa2b3 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 04:58:59 +0200 Subject: [PATCH 2/9] test(sarif): every finding must carry a confidence A fault observed while the program ran is certain (high); a vtable type mismatch may be a legitimate cast (medium); an unrecognized vtable report says little (low). The SARIF result must carry it as the confidence property, as CoreTrace's merged log spells it. Refs #30 --- unittests/sarif_test.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/unittests/sarif_test.cpp b/unittests/sarif_test.cpp index d835638..756f5d2 100644 --- a/unittests/sarif_test.cpp +++ b/unittests/sarif_test.cpp @@ -196,6 +196,28 @@ namespace rule + ": listed once, with a title and a description"); } } + + // How sure a finding is: a fault observed while the program ran is certain; a vtable type + // mismatch may be a legitimate cast; an unrecognized vtable report says little. + void EveryFindingHasAConfidence() + { + const std::vector> expected = { + {"heap-buffer-overflow", "high"}, + {"memory-leak", "high"}, + {"vtable-type-mismatch", "medium"}, + {"vtable-diagnostic", "low"}}; + for (const auto& [rule, confidence] : expected) + { + llvm::json::Object run = Run({OfRule(rule)}); + const llvm::json::Array* results = run.getArray("results"); + const llvm::json::Object* result = + results != nullptr && results->size() == 1 ? (*results)[0].getAsObject() : nullptr; + const llvm::json::Object* properties = + result != nullptr ? result->getObject("properties") : nullptr; + Expect(properties != nullptr && properties->getString("confidence") == confidence, + rule + ": confidence " + confidence); + } + } } // namespace int main() @@ -204,5 +226,6 @@ int main() FindingWithLocations(); WarningWithoutLocation(); EveryRuleIsDescribed(); + EveryFindingHasAConfidence(); return unittests::Finish("sarif_test"); } From 1fe9358b7efb347dff9712a5d976aa04ba3ebbc8 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 05:00:16 +0200 Subject: [PATCH 3/9] feat(findings): describe each rule the analyzer reports DescribeRule gives a title and a description for each of the eleven rules ParseFindings reports, next to where they are defined, for a SARIF consumer to explain their findings. Refs #29 --- src/findings.cpp | 54 ++++++++++++++++++++++++++++++++++++++++++++++++ src/findings.hpp | 11 ++++++++++ 2 files changed, 65 insertions(+) diff --git a/src/findings.cpp b/src/findings.cpp index 1375ccf..54a2f89 100644 --- a/src/findings.cpp +++ b/src/findings.cpp @@ -251,6 +251,60 @@ namespace coretrace::runtime_analyzer } } // namespace + std::optional 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."}}, + {"stack-buffer-overflow", + {"Stack buffer overflow", "A read or a write past the bounds of a stack object, " + "observed while the program ran."}}, + {"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."}}, + {"double-free", + {"Double free", "Heap memory freed a second time, observed while the program ran."}}, + {"memory-leak", + {"Memory leak", "Heap memory the program allocated and had not freed when it ended."}}, + {"vtable-null-this", + {"Virtual call through a null object", + "A virtual call made through a null pointer to an object."}}, + {"vtable-use-after-free", + {"Virtual call through a freed object", + "A virtual call made through an object whose memory was already freed."}}, + {"vtable-corrupted", + {"Virtual call through a corrupted vtable", + "A virtual call made through an object whose vtable pointer is missing, invalid " + "or without type information."}}, + {"vcall-invalid-target", + {"Virtual call to non-executable memory", + "A virtual call whose target address is not in executable memory."}}, + {"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."}}, + {"vtable-diagnostic", + {"Other virtual call issue", "A vtable or virtual call problem the runtime reported " + "without a more specific rule."}}, + }; + for (const DescribedRule& described : kRules) + { + if (described.rule == rule) + { + return described.description; + } + } + return std::nullopt; + } + std::vector ParseFindings(std::string_view output) { std::vector findings; diff --git a/src/findings.hpp b/src/findings.hpp index 678799a..c69b13c 100644 --- a/src/findings.hpp +++ b/src/findings.hpp @@ -4,6 +4,7 @@ #include "runtime_analyzer.hpp" +#include #include #include @@ -11,6 +12,16 @@ namespace coretrace::runtime_analyzer { // The memory errors the CoreTrace runtime reported in `output`, without ANSI escapes. [[nodiscard]] std::vector ParseFindings(std::string_view output); + + // What a rule checks, for a SARIF consumer to explain the rule's findings. + struct RuleDescription + { + std::string_view title; + std::string_view description; + }; + + // The description of `rule`; nothing for a rule this analyzer does not report. + [[nodiscard]] std::optional DescribeRule(std::string_view rule); } // namespace coretrace::runtime_analyzer #endif // CORETRACE_RUNTIME_ANALYZER_FINDINGS_HPP From d44af4902793b8d988705ad6dfa58da980f0cbd2 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 05:00:16 +0200 Subject: [PATCH 4/9] feat(sarif): list the reported rules with their title and description The driver had no rules, so a consumer could only show a finding's bare rule id. tool.driver.rules now lists each rule the run reports once, in the order first reported, with its title as shortDescription and its description as fullDescription. The exact logs the tests compare against list those rules, and an empty run an empty rules array. Refs #29 --- src/sarif.cpp | 25 ++++++++++++++++++++++++ unittests/sarif_test.cpp | 41 +++++++++++++++++++++++++++++++++------- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/src/sarif.cpp b/src/sarif.cpp index 45371da..0d597c9 100644 --- a/src/sarif.cpp +++ b/src/sarif.cpp @@ -1,10 +1,14 @@ // SPDX-License-Identifier: Apache-2.0 #include "sarif.hpp" +#include "findings.hpp" + #include #include #include +#include +#include #include namespace coretrace::runtime_analyzer @@ -27,6 +31,20 @@ 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 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 result{ @@ -53,14 +71,21 @@ namespace coretrace::runtime_analyzer void WriteSarif(std::ostream& out, const std::vector& findings) { llvm::json::Array results; + llvm::json::Array rules; + std::set 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)}}}, diff --git a/unittests/sarif_test.cpp b/unittests/sarif_test.cpp index 756f5d2..1f3b0db 100644 --- a/unittests/sarif_test.cpp +++ b/unittests/sarif_test.cpp @@ -31,11 +31,14 @@ namespace { "results": )"; - constexpr std::string_view kTail = R"(, + constexpr std::string_view kDriver = R"(, "tool": { "driver": { "informationUri": "https://github.com/CoreTrace/coretrace-runtime-analyzer", - "name": "coretrace-runtime-analyzer" + "name": "coretrace-runtime-analyzer", + "rules": )"; + + constexpr std::string_view kTail = R"( } } } @@ -44,14 +47,16 @@ namespace } )"; - [[nodiscard]] std::string Log(std::string_view results) + // The log of `results`, its driver listing `rules`. + [[nodiscard]] std::string Log(std::string_view results, std::string_view rules = "[]") { - return std::string(kHead) + std::string(results) + std::string(kTail); + return std::string(kHead) + std::string(results) + std::string(kDriver) + + std::string(rules) + std::string(kTail); } void EmptyLog() { - Expect(Sarif({}) == Log("[]"), "no finding: an empty results array"); + Expect(Sarif({}) == Log("[]"), "no finding: empty results and rules arrays"); } void FindingWithLocations() @@ -103,7 +108,18 @@ namespace ], "ruleId": "heap-buffer-overflow" } - ])"); + ])", + R"([ + { + "fullDescription": { + "text": "A read or a write past the bounds of a heap allocation, observed while the program ran." + }, + "id": "heap-buffer-overflow", + "shortDescription": { + "text": "Heap buffer overflow" + } + } + ])"); Expect(Sarif({finding}) == expected, "one error: location, allocation site as related location 0, rule, CWE"); } @@ -139,7 +155,18 @@ namespace ], "ruleId": "memory-leak" } - ])"); + ])", + R"([ + { + "fullDescription": { + "text": "Heap memory the program allocated and had not freed when it ended." + }, + "id": "memory-leak", + "shortDescription": { + "text": "Memory leak" + } + } + ])"); Expect(Sarif({finding}) == expected, "one warning: no locations, no column when the runtime gave none"); } From b336100049803f6f25507c776c60a1825ddd05be Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 05:00:58 +0200 Subject: [PATCH 5/9] feat(findings): rate how sure a finding of each rule is A fault observed while the program ran is certain (high); a vtable type mismatch may be a legitimate cast (medium); an unrecognized vtable report says little (low). Refs #30 --- src/findings.cpp | 42 +++++++++++++++++++++++++++--------------- src/findings.hpp | 4 +++- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/findings.cpp b/src/findings.cpp index 54a2f89..fb79823 100644 --- a/src/findings.cpp +++ b/src/findings.cpp @@ -262,38 +262,50 @@ namespace coretrace::runtime_analyzer // 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."}}, + {"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."}}, + {"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."}}, + {"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."}}, + {"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."}}, + {"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."}}, + "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."}}, + "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, invalid " - "or without type information."}}, + "or without type information.", + "high"}}, {"vcall-invalid-target", {"Virtual call to non-executable memory", - "A virtual call whose target address is not in 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."}}, + "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."}}, + {"Other virtual call issue", + "A vtable or virtual call problem the runtime reported " + "without a more specific rule.", + "low"}}, }; for (const DescribedRule& described : kRules) { diff --git a/src/findings.hpp b/src/findings.hpp index c69b13c..004e461 100644 --- a/src/findings.hpp +++ b/src/findings.hpp @@ -13,11 +13,13 @@ namespace coretrace::runtime_analyzer // The memory errors the CoreTrace runtime reported in `output`, without ANSI escapes. [[nodiscard]] std::vector ParseFindings(std::string_view output); - // What a rule checks, for a SARIF consumer to explain the rule's findings. + // 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. From 34b4a3efe0fd87637ad4fb3566404a25c3e2e3e5 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 05:00:58 +0200 Subject: [PATCH 6/9] feat(sarif): give each finding its confidence Each result carries its rule's confidence as the property confidence, low, medium or high, as CoreTrace's merged log spells it. The exact logs the tests compare against carry it next to the CWE. Refs #30 --- src/sarif.cpp | 7 ++++++- unittests/sarif_test.cpp | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/sarif.cpp b/src/sarif.cpp index 0d597c9..c7beb11 100644 --- a/src/sarif.cpp +++ b/src/sarif.cpp @@ -47,11 +47,16 @@ namespace coretrace::runtime_analyzer [[nodiscard]] llvm::json::Object Result(const Finding& finding) { + llvm::json::Object properties{{"cwe", finding.cwe}}; + if (const std::optional 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) { diff --git a/unittests/sarif_test.cpp b/unittests/sarif_test.cpp index 1f3b0db..543c590 100644 --- a/unittests/sarif_test.cpp +++ b/unittests/sarif_test.cpp @@ -87,6 +87,7 @@ namespace "text": "heap-buffer-overflow WRITE of size 4" }, "properties": { + "confidence": "high", "cwe": "CWE-122" }, "relatedLocations": [ @@ -135,6 +136,7 @@ namespace "text": "leak of 16 bytes" }, "properties": { + "confidence": "high", "cwe": "CWE-401" }, "relatedLocations": [ From 2f1d7c4db4d416a3aeb498d00f7a1b06630dc0ba Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 05:04:36 +0200 Subject: [PATCH 7/9] docs(readme): describe the rules and the confidence of the SARIF log Refs #29, #30 --- README.md | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 9c7d616..b880be5 100644 --- a/README.md +++ b/README.md @@ -125,9 +125,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 +145,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 +236,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`) | From 81e190083fa967d4c1e2e1c2d162f25cbf847d57 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 05:04:36 +0200 Subject: [PATCH 8/9] docs: announce the described rules and the confidence in the changelog Refs #29, #30 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e82af15..22840ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ 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`). ## v0.1.0 From 9cc2b8bfb2c50cb521c436888bd8a4b9a89ce1aa Mon Sep 17 00:00:00 2001 From: Hugo Date: Fri, 23 Oct 2026 02:57:15 +0200 Subject: [PATCH 9/9] fix(findings): classify a lone missing typeinfo as vtable-diagnostic A program built with -fno-rtti has no typeinfo in its vtables, so the runtime logs "missing typeinfo" for valid virtual calls. The parser mapped it to vtable-corrupted, an error rated high confidence. Move it last in the vtable rules as a vtable-diagnostic warning, so any other warning of the same box still decides the rule, and drop "type information" from the vtable-corrupted description. --- CHANGELOG.md | 2 ++ README.md | 7 +++++-- src/findings.cpp | 11 ++++++----- test/ct_vtable_diag_fake.cpp | 2 +- test/ct_vtable_no_rtti.cpp | 32 ++++++++++++++++++++++++++++++++ unittests/findings_test.cpp | 3 +++ 6 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 test/ct_vtable_no_rtti.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 22840ac..a2247a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ Notable changes are grouped by release. See [all releases](https://github.com/Co - 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 diff --git a/README.md b/README.md index b880be5..372939d 100644 --- a/README.md +++ b/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. diff --git a/src/findings.cpp b/src/findings.cpp index fb79823..77ff84d 100644 --- a/src/findings.cpp +++ b/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 │" @@ -290,8 +291,8 @@ namespace coretrace::runtime_analyzer "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, invalid " - "or without type information.", + "A virtual call made through an object whose vtable pointer is missing or " + "invalid.", "high"}}, {"vcall-invalid-target", {"Virtual call to non-executable memory", @@ -303,8 +304,8 @@ namespace coretrace::runtime_analyzer "medium"}}, {"vtable-diagnostic", {"Other virtual call issue", - "A vtable or virtual call problem the runtime reported " - "without a more specific rule.", + "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) diff --git a/test/ct_vtable_diag_fake.cpp b/test/ct_vtable_diag_fake.cpp index b8b0f56..1e43e22 100644 --- a/test/ct_vtable_diag_fake.cpp +++ b/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{}; diff --git a/test/ct_vtable_no_rtti.cpp b/test/ct_vtable_no_rtti.cpp new file mode 100644 index 0000000..9c58cdb --- /dev/null +++ b/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 + +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; +} diff --git a/unittests/findings_test.cpp b/unittests/findings_test.cpp index a59e9db..e2be70d 100644 --- a/unittests/findings_test.cpp +++ b/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) {