diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index 9dcdce05e..18b0aec73 100644 --- a/src/ReleaseHistory.md +++ b/src/ReleaseHistory.md @@ -3,6 +3,7 @@ ## Unreleased * FEATURE: Add `max-file-size-in-kb` argument that allows filtering scan targets by file size. [#2494](https://github.com/microsoft/sarif-sdk/pull/2494) +* BUGFIX: Fix false positive for `SARIF1002.UrisMustBeValid` for file URIs that omit the `authority`. [#2501](https://github.com/microsoft/sarif-sdk/pull/2501) ## **v2.4.15** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.4.15) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.4.15) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.4.15) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.4.15) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.4.15) diff --git a/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs b/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs index 52f8f2cf4..7848c5a88 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs @@ -80,7 +80,7 @@ private void AnalyzeUri(Uri uri, string pointer) string uriString = uri?.OriginalString; if (uriString != null) { - if (!Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) + if (!IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) { // {0}: The string '{1}' is not a valid URI reference. URIs must conform to // [RFC 3986](https://tools.ietf.org/html/rfc3986). diff --git a/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs b/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs index 26da20b12..3a92d3904 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs @@ -100,7 +100,7 @@ private void AnalyzeOriginalUriBaseIdsEntry(string uriBaseId, ArtifactLocation a // avoid having to do a try/catch. Unfortunately Uri.TryCreate will return true // even for a malformed URI string. string uriString = artifactLocation.Uri.OriginalString; - if (uriString != null && Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) + if (uriString != null && IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) { var uri = new Uri(uriString, UriKind.RelativeOrAbsolute); diff --git a/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs b/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs index b43b3cf54..541b1b1ba 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs @@ -77,7 +77,7 @@ private void AnalyzeUri(string uriString, string pointer) // Check for well-formedness first, before attempting to create a Uri object, to // avoid having to do a try/catch. Unfortunately Uri.TryCreate will return true // even for a malformed URI string. - if (uriString != null && Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) + if (uriString != null && IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) { // Ok, it's a well-formed URI of some kind. If it's not absolute, _now_ we // can report it. diff --git a/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs b/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs index 5f9ad2696..1bda2bd03 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs @@ -87,7 +87,7 @@ private void AnalyzeUri(string uriString, string pointer) // Check for well-formedness first, before attempting to create a Uri object, to // avoid having to do a try/catch. Unfortunately Uri.TryCreate will return true // even for a malformed URI string. - if (uriString != null && Uri.IsWellFormedUriString(uriString, UriKind.Absolute)) + if (uriString != null && IsWellFormedUriString(uriString, UriKind.Absolute)) { // Ok, it's a well-formed absolute URI. If it's not reachable, _now_ we can report it. Uri uri = new Uri(uriString, UriKind.Absolute); diff --git a/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs b/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs index 4da930a22..485fea970 100644 --- a/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs +++ b/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs @@ -246,6 +246,22 @@ internal static string JsonPointerToJavaScript(string pointerString) return sb.ToString(); } + /// + /// Validate URIs conform to [RFC 3986](https://tools.ietf.org/html/rfc3986). + /// + /// The string used to attempt to construct a `Uri`. + /// The type of the `Uri` in `uriString`. + /// + internal static bool IsWellFormedUriString(string uriString, UriKind uriKind) + { + bool isWellFormed = Uri.IsWellFormedUriString(uriString, uriKind); + + // `System.Uri.IsWellFormedUriString` incorrectly returns false if the `authority` is missing from the Uri string. + bool subjectToWellKnownClrBug = (uriString.StartsWith("file:/", StringComparison.OrdinalIgnoreCase) && Uri.TryCreate(uriString, uriKind, out Uri result)); + + return isWellFormed || subjectToWellKnownClrBug; + } + private static readonly string s_javaScriptIdentifierPattern = @"^[$_\p{L}][$_\p{L}0-9]*$"; private static readonly Regex s_javaScriptIdentifierRegex = new Regex(s_javaScriptIdentifierPattern, RegexOptions.Compiled); diff --git a/src/Sarif/Autogenerated/AddressComparer.cs b/src/Sarif/Autogenerated/AddressComparer.cs index fbe3185bb..dc192c71c 100644 --- a/src/Sarif/Autogenerated/AddressComparer.cs +++ b/src/Sarif/Autogenerated/AddressComparer.cs @@ -32,6 +32,8 @@ public int Compare(Address left, Address right) return compareResult; } + // TryReferenceCompares is an autogenerated extension method + // that will properly handle the case when 'left' is null. if (left.RelativeAddress.TryReferenceCompares(right.RelativeAddress, out compareResult)) { return compareResult; @@ -43,6 +45,8 @@ public int Compare(Address left, Address right) return compareResult; } + // TryReferenceCompares is an autogenerated extension method + // that will properly handle the case when 'left' is null. if (left.Length.TryReferenceCompares(right.Length, out compareResult)) { return compareResult; @@ -72,6 +76,8 @@ public int Compare(Address left, Address right) return compareResult; } + // TryReferenceCompares is an autogenerated extension method + // that will properly handle the case when 'left' is null. if (left.OffsetFromParent.TryReferenceCompares(right.OffsetFromParent, out compareResult)) { return compareResult; diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1002.UrisMustBeValid_Valid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1002.UrisMustBeValid_Valid.sarif index 301df35e2..14b9bb0fb 100644 --- a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1002.UrisMustBeValid_Valid.sarif +++ b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1002.UrisMustBeValid_Valid.sarif @@ -40,6 +40,12 @@ "uriBaseId": "SRCROOT", "index": 1 } + }, + { + "location": { + "uri": "file:/c:/src/file.c", + "index": 2 + } } ], "results": [ diff --git a/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs b/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs index 448e8dd71..3a4c10a9c 100644 --- a/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs +++ b/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; using System.Text; using FluentAssertions; @@ -76,5 +77,86 @@ public void SarifValidationSkimmerBase_JsonPointerToJavaScript_ProducesExpectedR string errorMessage = sb.ToString(); errorMessage.Should().BeEmpty(); } + + [Fact] + public void SarifValidationSkimmerBase_UriIsWellFormedUriString_ShouldReturnTrueIfWellFormedUri() + { + var testCases = new[] + { + new + { + uriKind = UriKind.Absolute, + uriString = "file:///c:/Code/sarif-sdk/src/" + }, + new + { + uriKind = UriKind.Absolute, + uriString = "https://example.com/my-project" + }, + new + { + uriKind = UriKind.Absolute, + uriString = "file:/c:/src/file.c" + }, + new + { + uriKind = UriKind.Absolute, + uriString = "file:/C:/src/file.c" + }, + new + { + uriKind = UriKind.Absolute, + uriString = "FILE:/C:/SRC/FILE.c" + }, + new + { + uriKind = UriKind.RelativeOrAbsolute, + uriString = "file:///c:/Code/sarif-sdk/src/" + }, + new + { + uriKind = UriKind.RelativeOrAbsolute, + uriString = "https://example.com/my-project" + }, + new + { + uriKind = UriKind.RelativeOrAbsolute, + uriString = "file:/c:/src/file.c" + }, + new + { + uriKind = UriKind.RelativeOrAbsolute, + uriString = "file://networkLocation/folder/someDoc.txt" + }, + }; + + foreach (var testCase in testCases) + { + SarifValidationSkimmerBase.IsWellFormedUriString(testCase.uriString, testCase.uriKind).Should().BeTrue(); + } + } + + [Fact] + public void SarifValidationSkimmerBase_UriIsWellFormedUriString_ShouldReturnFalseIfNotWellFormedUri() + { + var testCases = new[] + { + new + { + uriKind = UriKind.Absolute, + uriString = "ht%tp://www.example.com/rules/tst0001.html" + }, + new + { + uriKind = UriKind.RelativeOrAbsolute, + uriString = "ht%tp://www.example.com/rules/tst0001.html" + }, + }; + + foreach (var testCase in testCases) + { + SarifValidationSkimmerBase.IsWellFormedUriString(testCase.uriString, testCase.uriKind).Should().BeFalse(); + } + } } }