microsoft / sarif-visualstudio-extension

SARIF Microsoft Visual Studio Viewer Extension
MIT License
46 stars 19 forks source link

Adding codefinder functionality #596

Closed edkazcarlson-ms closed 1 year ago

edkazcarlson-ms commented 1 year ago

Currently the Sarif Viewer does no line matching, any highlights are placed purely based on the line number provided by the physical location.artifact location. This does not allow for code drift to be handled gracefully in case the code that a tool was ran on is not the exact same as a users local branch, which will cause the wrong line to be highlighted (some scenarios are: the tool is expensive/slow to run, the tool is not running on the code directly but on artifacts/crashes, etc). I have moved over the codefinding tool that we use on our extension, which relies on

  1. The "expected" line (the same as physicalLocation.region.startLine
  2. The text to match (region.snippet.text)
  3. The line to start searching from
  4. The line to stop searching
  5. (Optional) function name (LogicalLocation.fullyQualifiedName) Currently 1,2, and 5 are in the sarif spec already in some form. For the other 2 (requirements 3 and 4), I have added a new set of fields using the property bag of the locations object in order to denote the start and end line to search between. These are to be generated by the tool that generated the sarif log, as they are the most likely to understand the possible/expected range of drift.

I am especially open to changing the current location/name of the fields, I vaguely remember discussing before that we can have experimental fields that are not strictly a part of the sarif spec in order to prove their value, but I don't remember discussing exactly where these fields can go/ what they should be called.

edkazcarlson-ms commented 1 year ago

Example sarif file for testing:

{ "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", "version": "2.1.0", "runs": [ { "tool": { "driver": { "guid": "00000000-0000-4000-9000-000000000000", "name": "Demo", "shortDescription": { "text": "Random description" }, "fullName": "Demo Insight", "version": "1.0.0.0", "rules": [ { "id": "Rule-0", "shortDescription": { "text": "Random rule" } } ] } }, "originalUriBaseIds": { "%USERPROFILE%":{ "uri": "C:\Users\ecarlson" }, "GITHUBUSERCONTENT" : { "uri" : "https://raw.githubusercontent.com" } }, "results": [ { "ruleId": "Demo-0", "level": "error", "message": { "text": "Help me find \"switch (createStatus)\"" }, "locations": [ { "physicalLocation": { "artifactLocation": { "uri": "src/WebGoat.NET/AddNewUser.aspx.cs" }, "region": { "startLine": 40, "snippet": { "text": "switch (createStatus)" } }, "properties": { "StartLine": 35, "EndLine": 45 } }, "logicalLocations": [ { "fullyQualifiedName": "OWASP.WebGoat.NET.AddNewUser.CreateAccountButton_Click" } ] } ], "guid": "00000000-0000-4000-9000-000000000000" }, { "ruleId": "Demo-0", "level": "error", "message": { "text": "I'm alredy on the right line" }, "locations": [ { "physicalLocation": { "artifactLocation": { "uri": "src/WebGoat.NET/AddNewUser.aspx.cs" }, "region": { "startLine": 30, "snippet": { "text": "Membership.CreateUser(Username.Text, Password.Text," } }, "properties": { "StartLine": 20, "EndLine": 45 } }, "logicalLocations": [ { "fullyQualifiedName": "OWASP.WebGoat.NET.AddNewUser.CreateAccountButton_Click" } ] } ], "guid": "00000000-0000-4000-9000-000000000000" }, { "ruleId": "Demo-0", "level": "warning", "message": { "text": "Help me find \"SecurityQuestion.Text = passwordQuestion;\"" }, "locations": [ { "physicalLocation": { "artifactLocation": { "uri": "src/WebGoat.NET/AddNewUser.aspx.cs" }, "region": { "startLine": 21, "snippet": { "text": "SecurityQuestion.Text = passwordQuestion;" } }, "properties": { "StartLine": 15, "EndLine": 25 } }, "logicalLocations": [ { "fullyQualifiedName": "OWASP.WebGoat.NET.AddNewUser.Page_Load" } ] } ], "guid": "00000000-0000-4000-9000-000000000001" }, { "ruleId": "Demo-0", "level": "note", "message": { "text": "Help me find \"case MembershipCreateStatus.Success:\"" }, "locations": [ { "physicalLocation": { "artifactLocation": { "uri": "src/WebGoat.NET/AddNewUser.aspx.cs" }, "region": { "startLine": 44, "snippet": { "text": "case MembershipCreateStatus.Success:" } }, "properties": { "StartLine": 38, "EndLine": 50 } }, "logicalLocations": [ { "fullyQualifiedName": "OWASP.WebGoat.NET.AddNewUser.CreateAccountButton_Click" } ] } ], "guid": "00000000-0000-4000-9000-000000000002" }, { "ruleId": "Demo-0", "level": "note", "message": { "text": "Help me find a network file rooted with uriBaseId!" }, "locations": [ { "physicalLocation": { "artifactLocation": { "uri": "microsoft/sarif-visualstudio-extension/main/src/Sarif.Viewer.VisualStudio.Core/ErrorList/BaselineStateTableDataSource.cs", "uriBaseId": "GITHUBUSERCONTENT" }, "region": { "startLine": 20, "snippet": { "text": "_instance ?? (_instance = new BaselineStateTableDataSource());" } }, "properties": { "StartLine": 20, "EndLine": 30 } }, "logicalLocations": [ { "fullyQualifiedName": "Microsoft.Sarif.Viewer.ErrorList.BaselineStateTableDataSource" } ] } ], "guid": "00000000-0000-4000-9000-000000000003" }, { "ruleId": "Demo-0", "level": "note", "message": { "text": "Help me find a file rooted through BaseId" }, "locations": [ { "physicalLocation": { "artifactLocation": { "uri": "Guids.cs", "uriBaseId": "%USERPROFILE%" }, "region": { "startLine": 14, "snippet": { "text": "public static readonly Guid SariferCommandSet = new Guid(\"{CD8EE607-A630-4652-B2BA-748F534235C1}\");" } }, "properties": { "StartLine": 10, "EndLine": 15 } }, "logicalLocations": [ { "fullyQualifiedName": "Microsoft.CodeAnalysis.Sarif.Sarifer.Guids" } ] } ], "guid": "00000000-0000-4000-9000-000000000004" } ] } ] }

You need to change the %USERPROFILE% field under the originalUriBaseIds in order to properly find the Guid.cs file. For this example I used https://github.com/microsoft/sarif-visualstudio-extension/blob/main/src/Sarif.Sarifer.Core/Guids.cs

Untitled
edkazcarlson-ms commented 1 year ago

These changes also fix the https://github.com/microsoft/sarif-visualstudio-extension/issues/586 bug, as it will root files on-load instead of on-click, meaning it will prevent the regeneration or the error list items, which causes the new error list item to be placed on top