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

SARIF1010.RuleIdMustBeConsistent falsely claims ruleId property is absent #2799

Open
dvitek opened this issue Mar 7, 2024 · 0 comments
Open

Comments

@dvitek
Copy link

dvitek commented Mar 7, 2024

Here is a small sarif document with a whitespace rule id that would cause SARIF1010.RuleIdMustBeConsistent to erroneously claim that the ruleId property is absent:

1.sarif(17,17): error SARIF1010: runs[0].results[0]: This result contains neither of the properties 'ruleId' or 'rule.id'. The SARIF specification (§3.27.5) requires at least one of these properties to be present.

{
    "version": "2.1.0",
    "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
    "runs": [
        {
            "tool": {
                "driver": {
                    "name": "codeskanner",
                    "rules": [
                        {
                            "id": " "
                        }
                    ]
                }
            },
            "results": [
                {
                    "ruleId": " ",
                    "level": "warning",
                    "message" : {"text": "hello"}
                }]
        }
    ]
}

Specifically, according to the SARIF standard cited by the diagnostic (https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317643), empty string or whitespace only rule IDs are legal. Regardless of whether they are legal, the ruleId property is present.

While it usually would not be desirable to have such a silly ruleId (a space), we like to stress test our static analysis framework using a fuzzer. The fuzzer is good at exercising corner cases like this. We would like to be able to validate that the resulting sarif is standard-compliant, even if it is idiotic. This is the only false positive we are currently seeing from the validator.

SARIF1010.RuleIdMustBeConsistent.cs uses string.IsNullOrWhiteSpace to determine whether a ruleId property is absent.

if (string.IsNullOrWhiteSpace(result.RuleId) && string.IsNullOrWhiteSpace(result.Rule?.Id))

If SARIF1010.RuleIdMustBeConsistent is intending to enforce the SARIF standard, then I do not think string.IsNullOrWhiteSpace is the correct predicate. I rather suspect == null would be correct.

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

No branches or pull requests

1 participant