diff --git a/RoslynSecurityGuard.Test/RoslynSecurityGuard.Test.csproj b/RoslynSecurityGuard.Test/RoslynSecurityGuard.Test.csproj index 4e65178..80558d4 100644 --- a/RoslynSecurityGuard.Test/RoslynSecurityGuard.Test.csproj +++ b/RoslynSecurityGuard.Test/RoslynSecurityGuard.Test.csproj @@ -282,6 +282,7 @@ + diff --git a/RoslynSecurityGuard.Test/Tests/Taint/OpenRedirectAnalyzerTest.cs b/RoslynSecurityGuard.Test/Tests/Taint/OpenRedirectAnalyzerTest.cs new file mode 100644 index 0000000..d2b114e --- /dev/null +++ b/RoslynSecurityGuard.Test/Tests/Taint/OpenRedirectAnalyzerTest.cs @@ -0,0 +1,276 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using RoslynSecurityGuard.Analyzers.Taint; +using System.Collections.Generic; +using System.IO; +using System.Threading.Tasks; +using System.Xml; +using TestHelper; + +namespace RoslynSecurityGuard.Test.Tests.Taint +{ + [TestClass] + public class OpenRedirectAnalyzerTest : DiagnosticVerifier + { + + protected override IEnumerable GetDiagnosticAnalyzers() + { + return new[] { new TaintAnalyzer() }; + } + + protected override IEnumerable GetAdditionnalReferences() + { + return new[] { + MetadataReference.CreateFromFile(typeof(System.Web.HttpResponse).Assembly.Location), + MetadataReference.CreateFromFile(typeof(Microsoft.AspNetCore.Http.HttpResponse).Assembly.Location), + MetadataReference.CreateFromFile(typeof(System.Web.Mvc.ActionResult).Assembly.Location) + }; + } + + + [TestMethod] + public async Task OpenRedirectFound1() + { + var cSharpTest = @" +using System.Web; + +class OpenRedirect +{ + public static HttpResponse Response = null; + + public static void Run(string input) + { + Response.Redirect(""https://"" + input + ""/home.html""); + } +} +"; + var visualBasicTest = @" +Imports System.Web + +Class OpenRedirect + Public Shared Response As HttpResponse + + Public Shared Sub Run(input As String) + Response.Redirect(""https://"" + input + ""/home.html"") + End Sub +End Class +"; + var expected = new DiagnosticResult + { + Id = "SG0036", + Severity = DiagnosticSeverity.Warning, + }; + await VerifyCSharpDiagnostic(cSharpTest, expected); + await VerifyVisualBasicDiagnostic(visualBasicTest, expected); + } + + [TestMethod] + public async Task OpenRedirectFound2() + { + var cSharpTest = @" + using System.Web; + + class OpenRedirect + { + public static HttpResponse Response = null; + + public static void Run(string input) + { + Response.Redirect(input, true); + } + } + "; + var visualBasicTest = @" + Imports System.Web + + Class OpenRedirect + Public Shared Response As HttpResponse + + Public Shared Sub Run(input As String) + Response.Redirect(input, false) + End Sub + End Class + "; + var expected = new DiagnosticResult + { + Id = "SG0036", + Severity = DiagnosticSeverity.Warning, + }; + await VerifyCSharpDiagnostic(cSharpTest, expected); + await VerifyVisualBasicDiagnostic(visualBasicTest, expected); + } + + [TestMethod] + public async Task OpenRedirectFound3() + { + var cSharpTest = @" +using Microsoft.AspNetCore.Http; + +class OpenRedirect +{ + public static HttpResponse Response = null; + + public static void Run(string input) + { + Response.Redirect(input); + } +} +"; + var visualBasicTest = @" +Imports Microsoft.AspNetCore.Http + +Class OpenRedirect + Public Shared Response As HttpResponse + + Public Shared Sub Run(input As String) + Response.Redirect(input) + End Sub +End Class +"; + var expected = new DiagnosticResult + { + Id = "SG0036", + Severity = DiagnosticSeverity.Warning, + }; + await VerifyCSharpDiagnostic(cSharpTest, expected); + await VerifyVisualBasicDiagnostic(visualBasicTest, expected); + } + + [TestMethod] + public async Task OpenRedirectFound4() + { + var cSharpTest = @" + using Microsoft.AspNetCore.Http; + + class OpenRedirect + { + public static HttpResponse Response = null; + + public static void Run(string input) + { + Response.Redirect(input, true); + } + } + "; + var visualBasicTest = @" + Imports Microsoft.AspNetCore.Http + + Class OpenRedirect + Public Shared Response As HttpResponse + + Public Shared Sub Run(input As String) + Response.Redirect(input, false) + End Sub + End Class + "; + var expected = new DiagnosticResult + { + Id = "SG0036", + Severity = DiagnosticSeverity.Warning, + }; + await VerifyCSharpDiagnostic(cSharpTest, expected); + await VerifyVisualBasicDiagnostic(visualBasicTest, expected); + } + + [TestMethod] + public async Task OpenRedirectFound5() + { + var cSharpTest = @" +using System.Web.Mvc; + +class OpenRedirect : Controller +{ + public ActionResult Run(string input) + { + return Redirect(input); + } +} +"; + var visualBasicTest = @" +Imports System.Web.Mvc + +Public Class OpenRedirect + Inherits Controller + + Public Function Run(input As String) as ActionResult + Return Redirect(input) + End Function +End Class +"; + var expected = new DiagnosticResult + { + Id = "SG0036", + Severity = DiagnosticSeverity.Warning, + }; + await VerifyCSharpDiagnostic(cSharpTest, expected); + await VerifyVisualBasicDiagnostic(visualBasicTest, expected); + } + + [TestMethod] + public async Task OpenRedirectFound6() + { + var cSharpTest = @" +using System.Web.Mvc; + +class OpenRedirect : Controller +{ + public ActionResult Run(string input) + { + return RedirectPermanent(input); + } +} +"; + var visualBasicTest = @" +Imports System.Web.Mvc + +Public Class OpenRedirect + Inherits Controller + + Public Function Run(input As String) as ActionResult + Return RedirectPermanent(input) + End Function +End Class +"; + var expected = new DiagnosticResult + { + Id = "SG0036", + Severity = DiagnosticSeverity.Warning, + }; + await VerifyCSharpDiagnostic(cSharpTest, expected); + await VerifyVisualBasicDiagnostic(visualBasicTest, expected); + } + + [TestMethod] + public async Task OpenRedirectFalsePositive1() + { + var cSharpTest = @" +using System.Web; + +class OpenRedirect +{ + public static HttpResponse Response = null; + + public static void Run(string input) + { + Response.Redirect(""https://example.com/home.html""); + } +} +"; + var visualBasicTest = @" +Imports System.Web + +Class OpenRedirect + Public Shared Response As HttpResponse + + Public Shared Sub Run(input As String) + Response.Redirect(""https://example.com/home.html"") + End Sub +End Class +"; + + await VerifyCSharpDiagnostic(cSharpTest); + await VerifyVisualBasicDiagnostic(visualBasicTest); + } + } +} \ No newline at end of file diff --git a/RoslynSecurityGuard/Config/Messages.yml b/RoslynSecurityGuard/Config/Messages.yml index b2371b8..3c7f5a7 100644 --- a/RoslynSecurityGuard/Config/Messages.yml +++ b/RoslynSecurityGuard/Config/Messages.yml @@ -57,6 +57,11 @@ SG0003: title: Potential XPath injection with XmlDocument description: The dynamic value passed to the XPath query should be validated +SG0036: + class: TaintAnalyzer + title: Potential OpenRedirect + description: The dynamic value passed to the redirect should be validated + # Various SG0004: diff --git a/RoslynSecurityGuard/Config/Sinks.yml b/RoslynSecurityGuard/Config/Sinks.yml index 51be7ed..9659d7e 100644 --- a/RoslynSecurityGuard/Config/Sinks.yml +++ b/RoslynSecurityGuard/Config/Sinks.yml @@ -398,3 +398,60 @@ StreamReader_Create_path_encoding_bool_int: argTypes: (System.String, System.Text.Encoding, System.Boolean, System.Int32) injectableArguments: 0 locale: SG0018 + +# Open Redirects + +System.Web.HttpResponse_Redirect_string: + namespace: System.Web + className: HttpResponse + member: method + name: Redirect + argTypes: (System.String) + injectableArguments: 0 + locale: SG0036 + +System.Web.HttpResponse_Redirect_string_bool: + namespace: System.Web + className: HttpResponse + member: method + name: Redirect + argTypes: (System.String, System.Boolean) + injectableArguments: 0 + locale: SG0036 + +Microsoft.AspNetCore.Http.HttpResponse_Redirect_string: + namespace: Microsoft.AspNetCore.Http + className: HttpResponse + member: method + name: Redirect + argTypes: (System.String) + injectableArguments: 0 + locale: SG0036 + +Microsoft.AspNetCore.Http.HttpResponse_Redirect_string_bool: + namespace: Microsoft.AspNetCore.Http + className: HttpResponse + member: method + name: Redirect + argTypes: (System.String, System.Boolean) + injectableArguments: 0 + locale: SG0036 + + +Controller_Redirect_string: + namespace: System.Web.Mvc + className: Controller + member: method + name: Redirect + argTypes: (System.String) + injectableArguments: 0 + locale: SG0036 + +Controller_RedirectPermanent_string: + namespace: System.Web.Mvc + className: Controller + member: method + name: RedirectPermanent + argTypes: (System.String) + injectableArguments: 0 + locale: SG0036 \ No newline at end of file