BinSkim produced SARIF output is not recognized by github

[chipitsine]

SARIF is supposed to be first class citizen in github world.

however, github does not recognise BinSkim produced files, please have a look:

https://github.com/chipitsine/binskim-sarif-test

there was similar issue for DevSkim I really wonder how @gfs has found the root cause. logs are empty

@toshipiazza , can you please have a look ?

[chipitsine]

buildlogs

[gfs]

The GitHub requires the sarif to have some specific properties. Particularly the artifact location for findings needs to be a relative path to a file in the repository.

They've linked to the documentation here:

https://github.com/github/codeql-action/issues/665#issuecomment-889772747

You should be able to look at the "security scanning" section of your repo, it should have information about the last code scanning upload and will let you know if there was an error (but not exactly what it was). You can query the rest API to get more details about the latest analysis status (I have not tried this, I just followed the guidance from the comment above and it wotked).

[gfs]

If you provide the sarif here I may be able to identify what it is missing.

[chipitsine]

it is "ovpn.sarif" right in that repo: https://github.com/chipitsine/binskim-sarif-test

[gfs]

The BinSkim sarif is missing the following:

1. Message.Text is empty, the results do not contain a message.
2. artifact location.uri is absolute and not relative to the repo - however the secondary issue here is that binskim results are usually binaries which may be build artifact and not checked in so the uri won't match anything in the repo anyway
3. region information is blank - and also not really relevant to binaries but is required for github sarif issues.

https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#result-object

[michaelcfanning]

@eddynaka @yongyan-gh @shaopeng-gh @marmegh this is an important discussion to track.

@gfs has provided a thoughtful analysis. Generally, actions are designed to provide pointers to version-controlled things and typically, BinSkim is analyzing things that are produced by compilers or downloaded from a package store (i.e., not under version control). And, as Gabe notes, a BinSkim issue is at the file level, no region data is provided (there is a region mechanism for internal binary regions but BinSkim itself doesn't populate this, though it's an interesting idea that's been discussed).

Item #1 can be resolved by rewriting the SARIF log file to 'flatten' messages (i.e., transform message arguments and rule format strings to completely formed user-facing strings populated in message.text. FWIW, we can also use the multitool to make paths relative, but again these relative paths wouldn't point to things under source control (unless you contrived to check the scan targets into the repo).

Adding @josepalafox. Jose, any thoughts on this scenario? You have previously discussed the problem of scanning/reporting against content which isn't under source control, I'd guess.

[Porges]

Is there any progress on fixing this for 2.0?

[michaelcfanning]

@shaopeng-gh @yongyan-gh @EasyRhinoMSFT

Honestly, BinSkim should work well with GHAS. :) Team, can we please figure this out?

If GHAS still requires regions, my first question is does the SARIF validator tell us this? i.e., please run BinSkim SARIF through easy's validator.

If it does, we only have two options: update the SARIF GHAS converter to add a stub region, or update BinSkim itself to put a placeholder value in.

Someone should try to rewrite the SARIF file to produce a relative path. In other words, our first goal here is probably to contrive a working BinSkim file and then to figure out the best path to create that form.

[shaopeng-gh]

checking