Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FortifyFpr converter produces invalid SARIF #1593

Closed
michaelcfanning opened this issue Jul 26, 2019 · 1 comment
Closed

FortifyFpr converter produces invalid SARIF #1593

michaelcfanning opened this issue Jul 26, 2019 · 1 comment

Comments

@michaelcfanning
Copy link
Member

Convert Test.FunctionalTests.Sarif\v2\ConverterTestData\FortifyFpr\FortifyTest.fpr
Validate result

rule ids are missing, GUIDs don't validate, thread flow locations array is non-unique.

You can also run the UpdateBaselines.ps1 script to regen the current baseline to see the problem

@ghost ghost self-assigned this Aug 9, 2019
@ghost ghost changed the title Fortify converter produces invalid SARIF FortifyFpr converter produces invalid SARIF Aug 12, 2019
@ghost
Copy link

ghost commented Aug 12, 2019

  • The invalid GUIDs are the toolComponent id for the CWE taxonomy: it has only 11 digits in the last (12-digit) portion.

    This was due to an invalid GUID in FortifyFprConverter.CweToolComponent.Guid. Presumably this was a copy/paste error from when the author generated the GUID. I generated a new, valid GUID.

    Also, the invalid GUID had been copy/pasted to another location in the source code, rather than referring to it by its name FortifyFprConverter.CweToolComponent.Guid.

  • Some of the non-unique arrays are not threadFlowLocation objects; they are the string arrays in threadFlowLocation.kind, which contain the string "function" twice.

    This is due to an incorrect initialization of one of the elements of the ActionTypeToLocationKindMap (which I also renamed to ActionTypeToLocationKinds).

  • But the array run.threadFlowLocations does contains non-unique elements.

    Note that threadFlow.locations can contain non-unique elements (see §3.37.6), but run.threadFlowLocations cannot (see §3.14.19).

    The bug is a subtle one, due to an attempt to (according to a comment) "Serialize ThreadFlowLocations from the 'UnifiedNodePool' to maintain same reuse as Fortify log" This will need study.

  • Yes, rule.id really is missing. Fortify doesn't have anything other than the GUID to serve as rule.id -- which, per the spec, needs to be a "stable, opaque" identifier. So I'm going to assign the GUID to both the id and guid properties.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant