microsoft / microsoft/sarif-sdk

Handling Platform Specific Skimmers

Open
#751 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
226
Forks
108
Avg merge
3d 8h
Merged PRs (30d)
6

Description

@michaelcfanning, @lgolding for comments on approach, as I'm new to the Sarif-SDK internals.

Background: Porting BinSkim to run on Linux involves changing some skimmers to flag as "Only supported on Windows" since they rely on native interop. I've discussed this a bit offline with Michael--it seems like there's two options for marking the checks as not supported on a particular platform.

One option is changing the meaning of "CanAnalyze()" to also cover "Can't be analyzed on this platform"--however, the return of CanAnalyze seems to answer to "Should this binary be analyzed by this rule?", which is a different question from "Is this rule supported on this platform?", so this doesn't seem like the right approach. It also would be nice to give an actual error message (rather than just "can't analyze").

One good option is having every Skimmer that can't run on non-Windows platforms throw a PlatformNotSupportedException or similar, and disable the rule after the first time the exception is thrown. This has the nice effect of not spamming the user with "We can't run this rule" when analyzing large numbers of binaries, while still surfacing it as an error. However, this currently leads to a fatal exit code--so if we go this route, we'd want to also add a new non-fatal RunTimeError and treat that exception as a different case when Skimmer.Analyze is called in the AnalyzeCommandBase. (I've got what I believe are the appropriate changes in https://github.com/Microsoft/sarif-sdk/compare/master...Evmaus-MS:skimmer-platform-unsupported and can submit a PR for them if this is the correct approach.)

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Review the proposed changes in the linked skimmer-platform-unsupported comparison, then trace Skimmer.Analyze and AnalyzeCommandBase to understand how platform exceptions and exit codes are handled. Done means unsupported skimmers are surfaced with an explanatory non-fatal RunTimeError, disabled after the first exception, and no longer cause a fatal exit.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.