fix(sarif): read CWEs from the rule, and derive cci from the resolved nist - #8570
Open
clem-field wants to merge 1 commit into
Open
fix(sarif): read CWEs from the rule, and derive cci from the resolved nist#8570clem-field wants to merge 1 commit into
clem-field wants to merge 1 commit into
Conversation
… nist
Two defects in the SARIF mapper's tag resolution, with one root cause: each of
the three tags was computed independently from a different source.
CWEs were scraped out of the result message by splitting on the last '(' and
trimming two characters. That works only for producers whose message happens
to end in a parenthesised CWE list. For any other producer the scrape returned
a fragment of the prose as a "CWE", which then failed to map and silently fell
through to the default tags. SARIF publishes CWEs on the rule object, not the
result -- in properties.tags (Semgrep, CodeQL) and in
relationships[].target.id against a CWE taxonomy (the standards-blessed form,
which flawfinder uses). Rules are now indexed once per document and a result
resolves its own; the message is kept as a fallback for producers that
publish nothing on the rule.
The cci tag read `vulnerabilityClassifications`, a path SARIF does not define.
It therefore always resolved to nothing, and the value fell through to the
CCIs of the default tags -- so every control carried CCI-003173 and CCI-001643
(SA-11, RA-5) no matter what its own nist tag said. On the flawfinder sample
that meant 21 of 21 controls tagged nist SI-10 alongside CCIs belonging to two
unrelated controls. cci is now derived from the nist tags actually resolved
for that control.
Extraction is also strict: only CWE-<digits> tokens are accepted, so a message
with no identifier now yields no cwe tag rather than a fragment of prose.
Sample data changes on the existing flawfinder fixture: cci becomes
CCI-001310 (SI-10's actual CCI) on all 21 controls, and four controls have
their cwe list reordered because it now comes from the rule's relationships
rather than the message text. The CWE sets themselves are unchanged -- every
NIST control reachable from a CWE has an entry in the NIST-to-CCI table, so
no control loses tags it previously had.
Adds a real Semgrep SARIF sample as a second producer: its rules carry CWEs
in properties.tags and its messages carry none, which the previous
implementation could not read at all.
Signed-off-by: clem-field <kc8yhe@me.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.




Two defects in the SARIF mapper's tag resolution, with one root cause: each of the three tags was computed independently from a different source.
CWEs were scraped from the message text
extractCwesplit the finding message on the last(and trimmed two characters. That works only for producers whose message happens to end in a parenthesised CWE list. For any other producer it returned a fragment of the prose as a "CWE", which then failed to map and fell through to the default tags.Converting a real Semgrep SARIF against
mastertoday:SARIF publishes CWEs on the rule, not the result — in
properties.tags(Semgrep, CodeQL) and inrelationships[].target.idagainst a CWE taxonomy. Rules are now indexed once per document and a result resolves its own; the message is kept as a fallback for producers that publish nothing on the rule. After:"cwe": ["CWE-939"].Notably, flawfinder — the producer the old scrape appeared to handle — publishes its CWEs in
relationships[].target.idall along. The mapper never looked there.cci did not correspond to nist
The
ccitag readvulnerabilityClassifications, a path SARIF does not define. It always resolved to nothing, so the value fell through to the CCIs of the default tags. On the flawfinder sample that means 21 of 21 controls were taggednist: ["SI-10"]alongsidecci: ["CCI-003173", "CCI-001643"]— the CCIs for SA-11 and RA-5.cciis now derived from the nist tags actually resolved for that control, giving["CCI-001310"].Sample data changes
ccibecomesCCI-001310on all 21 flawfinder controls, and four controls have theircwelist reordered because it now comes from the rule's taxonomy relationships rather than the message text. The CWE sets themselves are unchanged, and every NIST control reachable from a CWE has an entry in the NIST-to-CCI table, so no control loses tags it previously had.Adds a real Semgrep SARIF as a second producer fixture: its rules carry CWEs in
properties.tagsand its messages carry none, which the previous implementation could not read at all.Prior art
sarif-to-hdfin mitre/hdf-libs already resolves CWEs this way — relationships first, thenproperties.tags, message text last, with cci derived from the resolved nist. This brings the v1 mapper in line with that design.Testing
Extraction is now strict: only
CWE-<digits>tokens are accepted, so a message with no identifier yields no cwe tag rather than a fragment of prose. Five new tests cover taxonomy relationships,properties.tags, the message fallback, the no-identifier case, and cci/nist agreement.vitest runinlibs/hdf-converters: 159 passing. The 4 failures (3 sonarqube, 1 splunk reverse) reproduce identically on an unmodifiedmastercheckout.