Skip to content

Commit 35c575a

Browse files
NettozxNettozx
committed
allow cwe tags for all rules and not just security related
1 parent 235f950 commit 35c575a

2 files changed

Lines changed: 76 additions & 59 deletions

File tree

‎cli/cppcheckexecutor.cpp‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,14 @@ namespace {
354354
// rule.properties.precision, rule.properties.problem.severity
355355
picojson::object properties;
356356
properties["precision"] = picojson::value(sarifPrecision(finding));
357+
358+
// Add CWE tag if available
359+
picojson::array tags;
360+
if (finding.cwe.id > 0)
361+
{
362+
tags.emplace_back(picojson::value("external/cwe/cwe-" + std::to_string(finding.cwe.id)));
363+
}
364+
357365
// Only set security-severity for findings that are actually security-related
358366
if (isSecurityRelatedFinding(finding.id))
359367
{
@@ -379,15 +387,16 @@ namespace {
379387
if (securitySeverity > 0.0)
380388
{
381389
properties["security-severity"] = picojson::value(std::to_string(securitySeverity));
382-
picojson::array tags{picojson::value("security")};
383-
// Add CWE tag if available
384-
if (finding.cwe.id > 0)
385-
{
386-
tags.emplace_back(picojson::value("external/cwe/cwe-" + std::to_string(finding.cwe.id)));
387-
}
388-
properties["tags"] = picojson::value(tags);
390+
tags.emplace_back(picojson::value("security"));
389391
}
390392
}
393+
394+
// Add tags array if it has any content
395+
if (!tags.empty())
396+
{
397+
properties["tags"] = picojson::value(tags);
398+
}
399+
391400
// Set problem.severity for use with github
392401
const std::string problemSeverity = sarifSeverity(finding);
393402
properties["problem.severity"] = picojson::value(problemSeverity);

‎test/testsarif.cpp‎

Lines changed: 60 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -621,80 +621,85 @@ int main() {
621621
const picojson::object& driver = tool.at("driver").get<picojson::object>();
622622
const picojson::array& rules = driver.at("rules").get<picojson::array>();
623623

624-
// Check that security-related rules have CWE tags
624+
// Check that rules with CWE IDs have CWE tags (regardless of security classification)
625625
bool foundNullPointerWithCwe = false;
626626
bool foundArrayBoundsWithCwe = false;
627627
bool foundMemleakWithCwe = false;
628628
bool foundDoubleFreeWithCwe = false;
629+
bool foundAnyCweTag = false;
629630

630631
for (const auto& rule : rules)
631632
{
632-
const picojson::object& r = rule.get<picojson::object>();
633-
const std::string ruleId = r.at("id").get<std::string>();
633+
const picojson::object& r = rule.get<picojson::object>();
634+
const std::string ruleId = r.at("id").get<std::string>();
635+
const picojson::object& props = r.at("properties").get<picojson::object>();
634636

635-
// Only check security-related rules that should have CWE tags
636-
if (ruleId == "nullPointer" || ruleId == "arrayIndexOutOfBounds" || ruleId == "doubleFree" ||
637-
ruleId == "memleak")
637+
// Check if this rule has tags
638+
if (props.find("tags") != props.end())
638639
{
639-
const picojson::object& props = r.at("properties").get<picojson::object>();
640+
const picojson::array& tags = props.at("tags").get<picojson::array>();
640641

641-
// Verify this rule has security-severity (prerequisite for CWE tags)
642-
if (props.find("security-severity") != props.end())
642+
bool hasSecurityTag = false;
643+
bool hasCweTag = false;
644+
std::string cweTag;
645+
646+
for (const auto& tag : tags)
643647
{
644-
// Should have tags array
645-
ASSERT(props.find("tags") != props.end());
646-
const picojson::array& tags = props.at("tags").get<picojson::array>();
648+
const std::string tagStr = tag.get<std::string>();
649+
if (tagStr == "security")
650+
{
651+
hasSecurityTag = true;
652+
}
653+
else if (tagStr.find("external/cwe/cwe-") == 0)
654+
{
655+
hasCweTag = true;
656+
foundAnyCweTag = true;
657+
cweTag = tagStr;
647658

648-
bool hasSecurityTag = false;
649-
bool hasCweTag = false;
650-
std::string cweTag;
659+
// Validate CWE tag format: external/cwe/cwe-<number>
660+
ASSERT_EQUALS(0, tagStr.find("external/cwe/cwe-"));
661+
std::string cweNumber = tagStr.substr(17); // After "external/cwe/cwe-"
662+
ASSERT(cweNumber.length() > 0);
651663

652-
for (const auto& tag : tags)
653-
{
654-
const std::string tagStr = tag.get<std::string>();
655-
if (tagStr == "security")
664+
// Verify it's a valid number
665+
for (char c : cweNumber)
656666
{
657-
hasSecurityTag = true;
667+
ASSERT(c >= '0' && c <= '9');
658668
}
659-
else if (tagStr.find("external/cwe/cwe-") == 0)
660-
{
661-
hasCweTag = true;
662-
cweTag = tagStr;
663-
664-
// Validate CWE tag format: external/cwe/cwe-<number>
665-
ASSERT_EQUALS(0, tagStr.find("external/cwe/cwe-"));
666-
std::string cweNumber = tagStr.substr(17); // After "external/cwe/cwe-"
667-
ASSERT(cweNumber.length() > 0);
668-
669-
// Verify it's a valid number
670-
for (char c : cweNumber)
671-
{
672-
ASSERT(c >= '0' && c <= '9');
673-
}
674669

675-
// Track specific CWE mappings we expect
676-
if (ruleId == "nullPointer" && cweNumber == "476")
677-
foundNullPointerWithCwe = true;
678-
else if (ruleId == "arrayIndexOutOfBounds" && cweNumber == "788")
679-
foundArrayBoundsWithCwe = true;
680-
else if (ruleId == "memleak" && cweNumber == "401")
681-
foundMemleakWithCwe = true;
682-
else if (ruleId == "doubleFree" && cweNumber == "415")
683-
foundDoubleFreeWithCwe = true;
684-
}
670+
// Track specific CWE mappings we expect for security-related rules
671+
if (ruleId == "nullPointer" && cweNumber == "476")
672+
foundNullPointerWithCwe = true;
673+
else if (ruleId == "arrayIndexOutOfBounds" && cweNumber == "788")
674+
foundArrayBoundsWithCwe = true;
675+
else if (ruleId == "memleak" && cweNumber == "401")
676+
foundMemleakWithCwe = true;
677+
else if (ruleId == "doubleFree" && cweNumber == "415")
678+
foundDoubleFreeWithCwe = true;
685679
}
680+
}
686681

687-
ASSERT_EQUALS(true, hasSecurityTag);
688-
ASSERT_EQUALS(true, hasCweTag);
682+
// If this is a security-related rule with CWE, it should have both security and CWE tags
683+
if (hasCweTag && (ruleId == "nullPointer" || ruleId == "arrayIndexOutOfBounds" ||
684+
ruleId == "doubleFree" || ruleId == "memleak"))
685+
{
686+
// Security-related rules should have security tag when they have security-severity
687+
if (props.find("security-severity") != props.end())
688+
{
689+
ASSERT_EQUALS(true, hasSecurityTag);
690+
}
689691
}
690692
}
691693
}
692694

693-
// Verify we found at least some of the expected CWE mappings
695+
// Verify we found at least some CWE tags (from any rules, not just security-related)
696+
ASSERT_EQUALS(true, foundAnyCweTag);
697+
698+
// Verify we found at least some of the expected security-related CWE mappings
694699
// Note: Not all may be present depending on what the test code triggers
695-
bool foundAnyCweMapping =
700+
bool foundSecurityCweMapping =
696701
foundNullPointerWithCwe || foundArrayBoundsWithCwe || foundMemleakWithCwe || foundDoubleFreeWithCwe;
697-
ASSERT_EQUALS(true, foundAnyCweMapping);
702+
ASSERT_EQUALS(true, foundSecurityCweMapping);
698703
}
699704

700705
void sarifRuleCoverage()
@@ -844,15 +849,18 @@ int main() {
844849
// Non-security rules should NOT have security-severity
845850
ASSERT(props.find("security-severity") == props.end());
846851

847-
// If they have tags, they should not include security or CWE tags
852+
// If they have tags, they should not include the security tag
853+
// but they MAY include CWE tags if they have CWE mappings
848854
if (props.find("tags") != props.end())
849855
{
850856
const picojson::array& tags = props.at("tags").get<picojson::array>();
851857
for (const auto& tag : tags)
852858
{
853859
const std::string tagStr = tag.get<std::string>();
860+
// Non-security rules should not have the security tag
854861
ASSERT(tagStr != "security");
855-
ASSERT(tagStr.find("external/cwe/cwe-") != 0);
862+
// But they may have CWE tags if they have CWE mappings
863+
// (no assertion against CWE tags here)
856864
}
857865
}
858866

0 commit comments

Comments
 (0)