oasis-tcs / sarif-spec

OASIS SARIF TC: Repository for development of the draft standard, where requests for modification should be made via Github Issues
https://github.com/oasis-tcs/sarif-spec
Other
168 stars 47 forks source link

Suggestion to add a "message" property to "threadFlowLocation" #556

Open Mcdostone opened 1 year ago

Mcdostone commented 1 year ago

I think it would be interesting to consider adding an optional message to a threadFlowLocation. By doing so, we allow tool makers to give more information and significance regarding the result.

Example

Taken from https://github.com/microsoft/sarif-tutorials/blob/main/docs/3-Beyond-basics.md#code-flows

{
  "version": "2.1.0",
  "runs": [
    {
      "tool": {
        "driver": {
          "name": "PythonScanner"
        }
      },
      "results": [
        {
          "ruleId": "PY2335",
          "message": {
            "text": "Use of tainted variable 'raw_input' in the insecure function 'eval'."
          },
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "3-Beyond-basics/bad-eval-with-code-flow.py"
                },
                "region": {
                  "startLine": 8
                }
              }
            }
          ],
          "codeFlows": [
            {
              "message": {
                "text": "Tracing the path from user input to insecure usage."
              },
              "threadFlows": [
                {
                  "locations": [
                    {
                      "message": { // New property 👀 
                        "text": "The tainted data enters the system here."
                      },
                      "location": {
                        "physicalLocation": {
                          "artifactLocation": {
                            "uri": "3-Beyond-basics/bad-eval-with-code-flow.py"
                          },
                          "region": {
                            "startLine": 3
                          }
                        }
                      }
                    },
                    {
                      "location": {
                        "physicalLocation": {
                          "artifactLocation": {
                            "uri": "3-Beyond-basics/bad-eval-with-code-flow.py"
                          },
                          "region": {
                            "startLine": 4
                          }
                        }
                      }
                    {
                      "message": {  // New property 👀
                        "text": "The tainted data is used insecurely here."
                      },
                      "location": {
                        "physicalLocation": {
                          "artifactLocation": {
                            "uri": "3-Beyond-basics/bad-eval-with-code-flow.py"
                          },
                          "region": {
                            "startLine": 8
                          }
                        }
                      }
                    }
                  ]
                }
              ]
            }
          ]
        }
      ]
    }
  ]
}

related PR

davidmalcolm commented 1 year ago

I wonder if you could achieve some of this via the threadFlowLocation "kinds" property (SARIF v2.1.0 section 3.38.8), perhaps with: ['acquire', 'taint'] and perhaps some new verb, say, 'use', for: ['use', 'taint']

davidmalcolm commented 1 year ago

See also https://github.com/oasis-tcs/sarif-spec/issues/530 for my suggestions about new "kinds" for that property, in case that's helpful.

michaelcfanning commented 1 year ago

David's suggestion is good to consider. More directly to your issue, wouldn't location.message do what you need?

                {
                  "locations": [
                    {
                        "location": {
                        "message": { // New property 👀 
                          "text": "The tainted data enters the system here."
                        },
                        "physicalLocation": {
                          "artifactLocation": {