From bc1f044c9605878e2360eb836de94fbd4f5aebef Mon Sep 17 00:00:00 2001 From: Dani Alcala <112832187+clavedeluna@users.noreply.github.com> Date: Thu, 9 May 2024 08:57:44 -0300 Subject: [PATCH] parse codeql results without region (#546) --- src/codemodder/codeql.py | 24 +++++------ tests/test_codeql.py | 91 ++++++++++++++++++++++++++++++---------- 2 files changed, 80 insertions(+), 35 deletions(-) diff --git a/src/codemodder/codeql.py b/src/codemodder/codeql.py index bfce9e82..4dcb174f 100644 --- a/src/codemodder/codeql.py +++ b/src/codemodder/codeql.py @@ -18,17 +18,19 @@ class CodeQLLocation(Location): def from_sarif(cls, sarif_location) -> Self: artifact_location = sarif_location["physicalLocation"]["artifactLocation"] file = Path(artifact_location["uri"]) - start = LineInfo( - line=sarif_location["physicalLocation"]["region"]["startLine"], - column=sarif_location["physicalLocation"]["region"].get("startColumn"), - ) + + try: + region = sarif_location["physicalLocation"]["region"] + except KeyError: + # A location without a region indicates a result for the entire file. + # Use sentinel values of 0 index for start/end + zero = LineInfo(0) + return cls(file=file, start=zero, end=zero) + + start = LineInfo(line=region["startLine"], column=region.get("startColumn")) end = LineInfo( - line=sarif_location["physicalLocation"]["region"].get( - "endLine", start.line - ), - column=sarif_location["physicalLocation"]["region"].get( - "endColumn", start.column - ), + line=region.get("endLine", start.line), + column=region.get("endColumn", start.column), ) return cls(file=file, start=start, end=end) @@ -40,7 +42,6 @@ def from_sarif( ) -> Self: extension_index = sarif_result["rule"]["toolComponent"]["index"] tool_index = sarif_result["rule"]["index"] - rule_data = rule_extensions[extension_index]["rules"][tool_index] locations: list[Location] = [] @@ -48,7 +49,6 @@ def from_sarif( try: codeql_location = CodeQLLocation.from_sarif(location) except KeyError: - # TODO: handle this case more gracefully continue locations.append(codeql_location) diff --git a/tests/test_codeql.py b/tests/test_codeql.py index 18575e48..5fc1be0c 100644 --- a/tests/test_codeql.py +++ b/tests/test_codeql.py @@ -8,13 +8,19 @@ class TestCodeQLResultSet(TestCase): def test_from_sarif(self): - # Given a SARIF file with known content sarif_content = { "runs": [ { "tool": { "driver": {"name": "CodeQL"}, - "extensions": [{"rules": [{"id": "python/sql-injection"}]}], + "extensions": [ + { + "rules": [ + {"id": "python/sql-injection"}, + {"id": "cs/web/missing-x-frame-options"}, + ] + }, + ], }, "results": [ { @@ -37,7 +43,28 @@ def test_from_sarif(self): "toolComponent": {"index": 0}, "index": 0, }, - } + }, + { + "ruleId": "cs/web/missing-x-frame-options", + "message": { + "text": "Configuration file is missing the X-Frame-Options setting." + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "index": 5, + "uri": "Web.config", + } + } + } + ], + "rule": { + "id": "cs/web/missing-x-frame-options", + "toolComponent": {"index": 0}, + "index": 1, + }, + }, ], } ] @@ -46,44 +73,62 @@ def test_from_sarif(self): with mock.patch( "builtins.open", mock.mock_open(read_data=json.dumps(sarif_content)) ): - # When parsing the SARIF file result_set = CodeQLResultSet.from_sarif(sarif_file) - # Then the result set should contain the expected results - self.assertEqual(len(result_set), 1) + self.assertEqual(len(result_set), 2) self.assertIn("python/sql-injection", result_set) - self.assertEqual(len(result_set["python/sql-injection"]), 1) + self.assertIn("cs/web/missing-x-frame-options", result_set) + + sql_result = result_set["python/sql-injection"] + self.assertEqual(len(sql_result), 1) self.assertEqual( - result_set["python/sql-injection"][Path("example.py")][0].rule_id, + sql_result[Path("example.py")][0].rule_id, "python/sql-injection", ) self.assertEqual( - result_set["python/sql-injection"][Path("example.py")][0] - .locations[0] - .file, + sql_result[Path("example.py")][0].locations[0].file, Path("example.py"), ) self.assertEqual( - result_set["python/sql-injection"][Path("example.py")][0] - .locations[0] - .start.line, + sql_result[Path("example.py")][0].locations[0].start.line, 10, ) self.assertEqual( - result_set["python/sql-injection"][Path("example.py")][0] - .locations[0] - .start.column, + sql_result[Path("example.py")][0].locations[0].start.column, 5, ) self.assertEqual( - result_set["python/sql-injection"][Path("example.py")][0] - .locations[0] - .end.line, + sql_result[Path("example.py")][0].locations[0].end.line, 10, ) self.assertEqual( - result_set["python/sql-injection"][Path("example.py")][0] - .locations[0] - .end.column, + sql_result[Path("example.py")][0].locations[0].end.column, 20, ) + + xframe_result = result_set["cs/web/missing-x-frame-options"] + self.assertEqual(len(xframe_result), 1) + self.assertEqual( + xframe_result[Path("Web.config")][0].rule_id, + "cs/web/missing-x-frame-options", + ) + self.assertEqual( + xframe_result[Path("Web.config")][0].locations[0].file, + Path("Web.config"), + ) + self.assertEqual( + xframe_result[Path("Web.config")][0].locations[0].start.line, + 0, + ) + self.assertEqual( + xframe_result[Path("Web.config")][0].locations[0].start.column, + -1, + ) + self.assertEqual( + xframe_result[Path("Web.config")][0].locations[0].end.line, + 0, + ) + self.assertEqual( + xframe_result[Path("Web.config")][0].locations[0].end.column, + -1, + )