diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/Remote.qll b/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/Remote.qll index 920bcac81bc3..7e0481c6fd4e 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/Remote.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/flowsources/Remote.qll @@ -11,6 +11,9 @@ private import semmle.code.csharp.frameworks.system.web.Services private import semmle.code.csharp.frameworks.system.web.ui.WebControls private import semmle.code.csharp.frameworks.WCF private import semmle.code.csharp.frameworks.microsoft.Owin +private import microsoft.code.csharp.frameworks.microsoft.Primitives +private import microsoft.code.csharp.frameworks.microsoft.AspNetCore + /** A data flow source of remote user input. */ abstract class RemoteFlowSource extends DataFlow::Node { @@ -187,3 +190,46 @@ class ActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode { override string getSourceType() { result = "ASP.NET MVC action method parameter" } } + +/** A data flow source of remote user input (ASP.NET Core). */ +abstract class AspNetCoreRemoteFlowSource extends RemoteFlowSource { } + + +/** A data flow source of remote user input (ASP.NET query collection). */ +class AspNetCoreQueryRemoteFlowSource extends AspNetCoreRemoteFlowSource, DataFlow::ExprNode { + AspNetCoreQueryRemoteFlowSource() { + exists(ValueOrRefType t, Call c, Access ac | + t instanceof MicrosoftAspNetCoreHttpHttpRequest or + t instanceof MicrosoftAspNetCoreHttpQueryCollection or + t instanceof MicrosoftAspNetCoreHttpQueryString | + this.getExpr() = c and + c.getTarget().getDeclaringType() = t + or + this.asExpr() = ac and + ac.getTarget().getDeclaringType() = t + or + c.getTarget().getDeclaringType().hasQualifiedName("Microsoft.AspNetCore.Http", "IQueryCollection") and + c.getTarget().getName() = "TryGetValue" and + this.asExpr() = c.getArgumentForName("value") + ) + } + + override + string getSourceType() { result = "ASP.NET Core query string" } +} + +/** + * A parameter to an Mvc controller action method, viewed as a source of remote user input. + */ +class AspNetCoreActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode { + AspNetCoreActionMethodParameter() { + exists(Parameter p | + p = this.getParameter() and + p.fromSource() | + p = any(MicrosoftAspNetCoreMvcController c).getAnActionMethod().getAParameter() + ) + } + + override + string getSourceType() { result = "ASP.NET Core MVC action method parameter" } +} diff --git a/csharp/ql/src/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll b/csharp/ql/src/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll new file mode 100644 index 000000000000..3699727dd0c1 --- /dev/null +++ b/csharp/ql/src/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll @@ -0,0 +1,396 @@ +/** + * Provides classes for working with Microsoft.AspNetCore.Mvc + */ + +import csharp +import semmle.code.csharp.frameworks.Microsoft + +class MicrosoftAspNetCoreNamespace extends Namespace { + MicrosoftAspNetCoreNamespace() { + getParentNamespace() instanceof MicrosoftNamespace and + hasName("AspNetCore") + } +} + +/** + * The `Microsoft.AspNetCore.Mvc` namespace. + */ +class MicrosoftAspNetCoreMvcNamespace extends Namespace { + MicrosoftAspNetCoreMvcNamespace() { + getParentNamespace() instanceof MicrosoftAspNetCoreNamespace and + hasName("Mvc") + } +} + +/** + * The 'Microsoft.AspNetCore.Mvc.ViewFeatures' namespace + */ +class MicrosoftAspNetCoreMvcViewFeatures extends Namespace { + MicrosoftAspNetCoreMvcViewFeatures() { + getParentNamespace() instanceof MicrosoftAspNetCoreMvcNamespace and + hasName("ViewFeatures") + } +} + +/** + * An attribute whose type is in the `Microsoft.AspNetCore.Mvc` namespace. + */ +class MicrosoftAspNetCoreMvcAttribute extends Attribute { + MicrosoftAspNetCoreMvcAttribute() { + getType().getNamespace() = any(MicrosoftAspNetCoreMvcNamespace mvc) + } +} + +/** + * An HttpPost attribute in `Microsoft.AspNetCore.Mvc` namespace + */ +class MicrosoftAspNetCoreMvcHttpPostAttribute extends MicrosoftAspNetCoreMvcAttribute { + MicrosoftAspNetCoreMvcHttpPostAttribute() { + getType().hasName("HttpPostAttribute") + } +} + +/** + * An HttpPut attribute in `Microsoft.AspNetCore.Mvc` namespace + */ +class MicrosoftAspNetCoreMvcHttpPutAttribute extends MicrosoftAspNetCoreMvcAttribute { + MicrosoftAspNetCoreMvcHttpPutAttribute() { + getType().hasName("HttpPutAttribute") + } +} + +/** + * An HttpDelete attribute in `Microsoft.AspNetCore.Mvc` namespace + */ +class MicrosoftAspNetCoreMvcHttpDeleteAttribute extends MicrosoftAspNetCoreMvcAttribute { + MicrosoftAspNetCoreMvcHttpDeleteAttribute() { + getType().hasName("HttpDeleteAttribute") + } +} + +/** + * An attribute whose type is `Microsoft.AspNetCore.Mvc.NonAction`. + */ +class MicrosoftAspNetCoreMvcNonActionAttribute extends MicrosoftAspNetCoreMvcAttribute { + MicrosoftAspNetCoreMvcNonActionAttribute() { + getType().hasName("NonActionAttribute") + } +} + +/** + * The `Microsoft.AspNetCore.Antiforgery` namespace. + */ +class MicrosoftAspNetCoreAntiforgeryNamespace extends Namespace { + MicrosoftAspNetCoreAntiforgeryNamespace() { + getParentNamespace() instanceof MicrosoftAspNetCoreNamespace and + hasName("Antiforgery") + } +} + +/** + * The `Microsoft.AspNetCore.Mvc.Filters` namespace. + */ +class MicrosoftAspNetCoreMvcFilters extends Namespace { + MicrosoftAspNetCoreMvcFilters() { + getParentNamespace() instanceof MicrosoftAspNetCoreMvcNamespace and + hasName("Filters") + } +} + +/** + * The `Microsoft.AspNetCore.Mvc.Filters.IFilterMetadataInterface` interface. + */ +class MicrosoftAspNetCoreMvcIFilterMetadataInterface extends Interface { + MicrosoftAspNetCoreMvcIFilterMetadataInterface() { + getNamespace() = any(MicrosoftAspNetCoreMvcFilters h) and + hasName("IFilterMetadata") + } +} + + /** + * The `Microsoft.AspNetCore.IAuthorizationFilter` interface. + */ +class MicrosoftAspNetCoreIAuthorizationFilterInterface extends Interface { + MicrosoftAspNetCoreIAuthorizationFilterInterface() { + getNamespace() = any(MicrosoftAspNetCoreMvcFilters h) and + hasName("IAsyncAuthorizationFilter") + } + + Method getOnAuthorizationMethod() { + result = getAMethod("OnAuthorizationAsync") + } +} + +/** + * The `Microsoft.AspNetCore.IAntiforgery` interface. + */ +class MicrosoftAspNetCoreIAntiForgeryInterface extends Interface { + MicrosoftAspNetCoreIAntiForgeryInterface() { + getNamespace() = any(MicrosoftAspNetCoreAntiforgeryNamespace h) and + hasName("IAntiforgery") + } + + /** + * Gets the `ValidateRequestAsync` method. + */ + Method getValidateMethod() { + result = getAMethod("ValidateRequestAsync") + } +} + +/** + * The `Microsoft.AspNetCore.DefaultAntiForgery` class, or another user-supplied class that implements IAntiForgery + */ +class AntiForgeryClass extends Class { + AntiForgeryClass () { + getABaseInterface*() instanceof MicrosoftAspNetCoreIAntiForgeryInterface + } + + /** + * Gets the `ValidateRequestAsync` method. + */ + Method getValidateMethod() { + result = getAMethod("ValidateRequestAsync") + } +} + +/** + * Authorization filter class defined by AspNetCore or the user + */ +class AuthorizationFilterClass extends Class { + AuthorizationFilterClass() { + getABaseInterface*() instanceof MicrosoftAspNetCoreIAuthorizationFilterInterface + } + + /** Gets the `OnAuthorization` method provided by this filter. */ + Method getOnAuthorizationMethod() { + result = getAMethod("OnAuthorizationAsync") + } +} + +/** + * An attribute whose type has a name like `[Auto...]Validate[...]Anti[Ff]orgery[...Token]Attribute`. + */ +class ValidateAntiForgeryAttribute extends Attribute { + ValidateAntiForgeryAttribute() { + getType().getName().matches("%Validate%Anti_orgery%Attribute") + } +} + +/** + * An class that has a name like `[Auto...]Validate[...]Anti[Ff]orgery[...Token]` and implements IFilterMetadata interface + * This class can be added to a collection of global MvcOptions.Filters collection + */ +class ValidateAntiforgeryTokenAuthorizationFilter extends Class { + ValidateAntiforgeryTokenAuthorizationFilter() { + getABaseInterface*() instanceof MicrosoftAspNetCoreMvcIFilterMetadataInterface and + getName().matches("%Validate%Anti_orgery%") + } +} + +/** + * The `Microsoft.AspNetCore.Mvc.Filters.FilterCollection` class + */ +class MicrosoftAspNetCoreMvcFilterCollection extends Class { + MicrosoftAspNetCoreMvcFilterCollection() { + getNamespace() = any(MicrosoftAspNetCoreMvcFilters h) and + hasName("FilterCollection") + } + + Method getAddMethod() { + result = getAMethod("Add") or + result = getABaseType().getAMethod("Add") + } +} + +/** + * The `Microsoft.AspNetCore.Mvc.MvcOptions` class . + */ +class MicrosoftAspNetCoreMvcOptions extends Class { + MicrosoftAspNetCoreMvcOptions() { + getNamespace() = any(MicrosoftAspNetCoreMvcNamespace mvc) and + hasName("MvcOptions") + } + + Property getFilterCollectionProperty() { + result = getProperty("Filters") + } +} + +/** + * The base class for controllers in MVC, i.e. `Microsoft.AspNetCore.Mvc.Controller` or `Microsoft.AspNetCore.Mvc.ControllerBase` class. + */ + +class MicrosoftAspNetCoreMvcControllerBaseClass extends Class { + MicrosoftAspNetCoreMvcControllerBaseClass() { + getNamespace() = any(MicrosoftAspNetCoreMvcNamespace h) and + (hasName("Controller") or hasName("ControllerBase")) + } +} + +/** + * An subtype of `Microsoft.AspNetCore.Mvc.Controller` or `Microsoft.AspNetCore.Mvc.ControllerBase`. + */ +class MicrosoftAspNetCoreMvcController extends Class { + MicrosoftAspNetCoreMvcController() { + getABaseType*() instanceof MicrosoftAspNetCoreMvcControllerBaseClass + } + + /** Gets an action method for this controller. */ + Method getAnActionMethod() { + result = getAMethod() and + result.isPublic() and + not result.isStatic() and + not result.getAnAttribute() instanceof MicrosoftAspNetCoreMvcNonActionAttribute + } + + /** + * Gets an "action" method handling POST, PUT and DELETE request, which may be called by the MVC framework in response to a user + * request. + */ + Method getAnActionModifyingMethod() { + result = getAnActionMethod() and + (result.getAnAttribute() instanceof MicrosoftAspNetCoreMvcHttpPostAttribute or + result.getAnAttribute() instanceof MicrosoftAspNetCoreMvcHttpPutAttribute or + result.getAnAttribute() instanceof MicrosoftAspNetCoreMvcHttpDeleteAttribute) + } + + /** Gets a `Redirect..` method. */ + Method getARedirectMethod() { + result = this.getAMethod() and + result.getName().matches("Redirect%") + } +} + +/* + * Returns a string corresponding to an HTTP method used in the action method + */ +string httpMethodType(Method m) { + m.getAnAttribute() instanceof MicrosoftAspNetCoreMvcHttpPostAttribute and result = "POST" or + m.getAnAttribute() instanceof MicrosoftAspNetCoreMvcHttpPutAttribute and result = "PUT" or + m.getAnAttribute() instanceof MicrosoftAspNetCoreMvcHttpDeleteAttribute and result = "DELETE" + } + +/** + * The `Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper` class. + */ +class MicrosoftAspNetCoreMvcHtmlHelperClass extends Class { + MicrosoftAspNetCoreMvcHtmlHelperClass() { + getNamespace() = any(MicrosoftAspNetCoreMvcViewFeatures mvc) and + hasName("HtmlHelper") + } + + /** Gets the `Raw` method. */ + Method getRawMethod() { + result = getAMethod("Raw") + } +} + +/** + * Class deriving from Microsoft.AspNetCore.Mvc.Razor.RazorPageBase, implements Razor page in ASPNET Core + */ +class MicrosoftAspNetCoreMvcRazorPageBase extends Class { + MicrosoftAspNetCoreMvcRazorPageBase () { + this.getABaseType*().hasQualifiedName("Microsoft.AspNetCore.Mvc.Razor", "RazorPageBase") + } + + Method getWriteLiteralMethod() { + result = getAMethod("WriteLiteral") + } +} + +/** + * Class deriving from Microsoft.AspNetCore.Http.HttpRequest, implements HttpRequest in ASPNET Core + */ +class MicrosoftAspNetCoreHttpHttpRequest extends Class { + MicrosoftAspNetCoreHttpHttpRequest() { + this.getABaseType*().hasQualifiedName("Microsoft.AspNetCore.Http", "HttpRequest") + } +} + +/** + * Class deriving from Microsoft.AspNetCore.Http.HttpResponse, implements HttpResponse in ASPNET Core + */ +class MicrosoftAspNetCoreHttpHttpResponse extends Class { + MicrosoftAspNetCoreHttpHttpResponse() { + this.getABaseType*().hasQualifiedName("Microsoft.AspNetCore.Http", "HttpResponse") + } + + Method getRedirectMethod() { + result = this.getAMethod("Redirect") + } + + Property getHeadersProperty() { + result = this.getProperty("Headers") + } +} + +/** + * Class is a wrapper around the collection of cookies in the response + */ +class MicrosoftAspNetCoreHttpResponseCookies extends Interface{ + MicrosoftAspNetCoreHttpResponseCookies() { + this.hasQualifiedName("Microsoft.AspNetCore.Http.IResponseCookies") + } + + Method getAppendMethod() { + result = this.getAMethod("Append") + } +} + +/** + * Class Microsoft.AspNetCore.Http.QueryString, holds query string in ASP.NET Core + */ +class MicrosoftAspNetCoreHttpQueryString extends Struct { + MicrosoftAspNetCoreHttpQueryString() { + this.hasQualifiedName("Microsoft.AspNetCore.Http", "QueryString") + } +} + +/** + * Class implementing IQueryCollection, holds parsed query string in ASP.NET Core + */ +class MicrosoftAspNetCoreHttpQueryCollection extends RefType { + MicrosoftAspNetCoreHttpQueryCollection() { + this.getABaseInterface().hasQualifiedName("Microsoft.AspNetCore.Http", "IQueryCollection") + } +} + +/** + * Helper class for setting headers + */ +class MicrosoftAspNetCoreHttpResponseHeaders extends RefType { + MicrosoftAspNetCoreHttpResponseHeaders() { + this.hasQualifiedName("Microsoft.AspNetCore.Http.Headers","ResponseHeaders") + } + + Property getLocationProperty() { + result = this.getProperty("Location") + } +} + +class MicrosoftAspNetCoreHttpHeaderDictionaryExtensions extends RefType { + MicrosoftAspNetCoreHttpHeaderDictionaryExtensions () { + this.hasQualifiedName("Microsoft.AspNetCore.Http","HeaderDictionaryExtensions") + } + + Method getAppendMethod() { + result = this.getAMethod("Append") + } + + Method getAppendCommaSeparatedValuesMethod() { + result = this.getAMethod("AppendCommaSeparatedValues") + } + + Method getSetCommaSeparatedValuesMethod() { + result = this.getAMethod("SetCommaSeparatedValues") + } +} +/** + * Class Microsoft.AspNetCore.Html.HtmlString, suppose to wrap HTML-encoded string in ASP.NET Core + * Untrusted and unsanitized data should never flow there. + */ +class MicrosoftAspNetCoreHttpHtmlString extends Class { + MicrosoftAspNetCoreHttpHtmlString() { + this.hasQualifiedName("Microsoft.AspNetCore.Html", "HtmlString") + }} diff --git a/csharp/ql/src/semmle/code/csharp/frameworks/microsoft/Primitives.qll b/csharp/ql/src/semmle/code/csharp/frameworks/microsoft/Primitives.qll new file mode 100644 index 000000000000..603672c08d0c --- /dev/null +++ b/csharp/ql/src/semmle/code/csharp/frameworks/microsoft/Primitives.qll @@ -0,0 +1,32 @@ +/** + * Provides definitions for working with classes in Microsoft.Extensions.Primitives namespace + */ +import csharp + +module ExtensionPrimitives { +private import semmle.code.csharp.dataflow.LibraryTypeDataFlow + +/** + * StringValues class used in many .Net Core libraries. Requreres special LibraryTypeDataFlow flow. + */ + class StringValues extends Struct { + StringValues() { + this.hasQualifiedName("Microsoft.Extensions.Primitives", "StringValues") + } + } + + /** + * Custom flow through StringValues.StringValues library class + */ + class StringValuesFlow extends LibraryTypeDataFlow, StringValues { + override predicate callableFlow(CallableFlowSource source, CallableFlowSink sink, SourceDeclarationCallable c, boolean preservesValue) { + c = any(Callable ca | this = ca.getDeclaringType()) + and + (source = any(CallableFlowSourceArg a) or + source = any(CallableFlowSourceQualifier q)) + and + sink = any(CallableFlowSinkReturn r) + and preservesValue = false + } + } +} \ No newline at end of file diff --git a/csharp/ql/src/semmle/code/csharp/frameworks/system/web/WebPages.qll b/csharp/ql/src/semmle/code/csharp/frameworks/system/web/WebPages.qll new file mode 100644 index 000000000000..8fcfe741bd58 --- /dev/null +++ b/csharp/ql/src/semmle/code/csharp/frameworks/system/web/WebPages.qll @@ -0,0 +1,37 @@ +/** Definitions related to the namespace `System.Web.WebPages`, ASP.NET */ +import csharp + +private import semmle.code.csharp.frameworks.system.Web + +/** The `System.Web.WebPages` namespace. */ +class SystemWebWebPagesNamespace extends Namespace { + SystemWebWebPagesNamespace() { + getParentNamespace() instanceof SystemWebNamespace and + hasName("WebPages") + } +} + +/** The `System.Web.WebPages.WebPageExecutingBase` class. */ +class SystemWebWebPagesWebPageExecutingBaseClass extends Class { + SystemWebWebPagesWebPageExecutingBaseClass() { + getNamespace() instanceof SystemWebWebPagesNamespace and + hasName("WebPageExecutingBase") + } +} + +/** + * This class describes any class that derives from System.Web.WebPages.WebPageExecutingBase + */ +class WebPageClass extends Class { + WebPageClass () { + this.getBaseClass*() instanceof SystemWebWebPagesWebPageExecutingBaseClass + } + + Method getWriteLiteralMethod() { + result = getAMethod("WriteLiteral") + } + + Method getWriteLiteralToMethod() { + result = getAMethod("WriteLiteralTo") + } +} \ No newline at end of file diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/UrlRedirect.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/UrlRedirect.qll index 5ef7a9d7b628..3183b975eaa3 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/UrlRedirect.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/UrlRedirect.qll @@ -9,6 +9,7 @@ module UrlRedirect { import semmle.code.csharp.frameworks.system.Web import semmle.code.csharp.frameworks.system.web.Mvc import semmle.code.csharp.security.Sanitizers + import semmle.code.csharp.frameworks.microsoft.AspNetCore /** * A data flow source for unvalidated URL redirect vulnerabilities. @@ -149,4 +150,65 @@ module UrlRedirect { private class SimpleTypeSanitizer extends Sanitizer, SimpleTypeSanitizedExpr { } private class GuidSanitizer extends Sanitizer, GuidSanitizedExpr { } + + /** + * A URL argument to a call to `HttpResponse.Redirect()` or `Controller.Redirect()`, that is a + * sink for URL redirects. + */ + class AspNetCoreRedirectSink extends Sink { + AspNetCoreRedirectSink() { + exists(MethodCall mc | + mc.getTarget() = any(MicrosoftAspNetCoreHttpHttpResponse response).getRedirectMethod() or + mc.getTarget() = any(MicrosoftAspNetCoreMvcController response).getARedirectMethod() + | + // Response.Redirect uses 'location' parameter + this.getExpr() = mc.getArgumentForName("location") or + // Redirect uses the parameter name 'url' + this.getExpr() = mc.getArgumentForName("url") or + // Controller.RedirectToAction* + this.getExpr() = mc.getArgumentForName("actionName") or + // Controller.RedirectToRoute* + this.getExpr() = mc.getArgumentForName("routeName") or + // Controller.RedirectToPage* + this.getExpr() = mc.getArgumentForName("pageName") + ) + } + } + + /** + * Anything that is setting "location" header in the response headers. + */ + class AspNetCoreLocationHeaderSink extends Sink { + AspNetCoreLocationHeaderSink () { + // ResponseHeaders.Location = + exists(Assignment propAssign, PropertyAccess pa | + pa.getTarget() = any(MicrosoftAspNetCoreHttpResponseHeaders headers).getLocationProperty() and + pa = propAssign.getLValue() and + this.asExpr() = propAssign.getRValue()) + or // HttpResponse.Headers.Append("location", ) + exists(MethodCall mc, MicrosoftAspNetCoreHttpHeaderDictionaryExtensions ext | + mc.getTarget() = ext.getAppendMethod() or + mc.getTarget() = ext.getAppendCommaSeparatedValuesMethod() or + mc.getTarget() = ext.getSetCommaSeparatedValuesMethod() | + mc.getArgumentForName("key").getValue().toLowerCase() = "location" and + this.getExpr() = mc.getArgument(2)) + or // HttpResponse.Headers.Add("location", ) + exists(RefType cl, MicrosoftAspNetCoreHttpHttpResponse resp, PropertyAccess c, MethodCall add | + c.getTarget() = resp.getHeadersProperty() and + add.getTarget() = cl.getAMethod("Add") and + c = add.getQualifier() and + add.getArgument(0).getValue().toLowerCase() = "location" and + this.getExpr() = add.getArgument(1)) + or // HttpResponse.Headers["location"] = + exists(RefType cl, MicrosoftAspNetCoreHttpHttpResponse resp, IndexerAccess ci, Call cs, PropertyAccess pa | + pa.getTarget() = resp.getHeadersProperty() and + ci.getTarget() = cl.getAnIndexer() and + pa = ci.getQualifier() and + cs.getTarget() = cl.getAnIndexer().getSetter() and + cs.getArgument(0).getValue().toLowerCase() = "location" and + this.asExpr() = cs.getArgument(1)) + } + } } + + diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll index 151cc68c71c4..1c00812cb3a3 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll @@ -9,11 +9,13 @@ module XSS { import semmle.code.csharp.frameworks.system.Net import semmle.code.csharp.frameworks.system.Web import semmle.code.csharp.frameworks.system.web.Mvc + import semmle.code.csharp.frameworks.system.web.WebPages import semmle.code.csharp.frameworks.system.web.UI import semmle.code.csharp.frameworks.system.web.ui.WebControls import semmle.code.csharp.frameworks.system.windows.Forms import semmle.code.csharp.security.Sanitizers import semmle.code.asp.AspNet + import semmle.code.csharp.frameworks.microsoft.AspNetCore /** * Holds if there is tainted flow from `source` to `sink` that may lead to a @@ -514,6 +516,75 @@ module XSS { this.getExpr() = any(ObjectCreation oc | oc.getTarget().getDeclaringType().hasQualifiedName("System.Net.Http", "StringContent")).getArgumentForName("content") } } + + /** + * An expression that is used as an argument to `Page.WriteLiteral`, typically in + * a `.cshtml` file. + */ + class WebPageWriteLiteralSink extends Sink, HtmlSink { + WebPageWriteLiteralSink() { + this.getExpr() = any(WebPageClass h).getWriteLiteralMethod().getACall().getAnArgument() + } + + override string explanation() { + result = "System.Web.WebPages.WebPage.WriteLiteral() method" + } + } + + /** + * An expression that is used as an argument to `Page.WriteLiteralTo`, typically in + * a `.cshtml` file. + */ + class WebPageWriteLiteralToSink extends Sink, HtmlSink { + WebPageWriteLiteralToSink() { + this.getExpr() = any(WebPageClass h).getWriteLiteralToMethod().getACall().getAnArgument() + } + + override string explanation() { + result = "System.Web.WebPages.WebPage.WriteLiteralTo() method" + } + } + + abstract class AspNetCoreSink extends Sink, HtmlSink { } + + /** + * An expression that is used as an argument to `HtmlHelper.Raw`, typically in + * a `.cshtml` file. + */ + class MicrosoftAspNetCoreMvcHtmlHelperRawSink extends AspNetCoreSink { + MicrosoftAspNetCoreMvcHtmlHelperRawSink() { + this.getExpr() = any(MicrosoftAspNetCoreMvcHtmlHelperClass h).getRawMethod().getACall().getAnArgument() + } + + override string explanation() { + result = "Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper.Raw() method" + } + } + + /** + * An expression that is used as an argument to `Page.WriteLiteral` in ASP.NET 6.0 razor page, typically in + * a `.cshtml` file. + */ + class MicrosoftAspNetRazorPageWriteLiteralSink extends AspNetCoreSink { + MicrosoftAspNetRazorPageWriteLiteralSink() { + this.getExpr() = any(MicrosoftAspNetCoreMvcRazorPageBase h).getWriteLiteralMethod().getACall().getAnArgument() + } + + override string explanation() { + result = "Microsoft.AspNetCore.Mvc.Razor.RazorPageBase.WriteLiteral() method" + } + } + + /** + * HtmlString that may be rendered as is need to have sanitized value + */ + class MicrosoftAspNetHtmlStringSink extends AspNetCoreSink { + MicrosoftAspNetHtmlStringSink() { + exists (ObjectCreation c, MicrosoftAspNetCoreHttpHtmlString s | + c.getTarget() = s.getAConstructor() and + this.asExpr() = c.getAnArgument()) + } + } } private Type getMemberType(Member m) { diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNet/XSS.cs b/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNet/XSS.cs new file mode 100644 index 000000000000..887106fbcba9 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNet/XSS.cs @@ -0,0 +1,50 @@ +// semmle-extractor-options: /r:System.Web.dll /r:${testdir}/../../../../../packages/Microsoft.AspNet.WebPages.3.2.3/lib/net45/System.Web.WebPages.dll /r:${testdir}/../../../../../packages/Microsoft.AspNet.Mvc.5.2.3/lib/net45/System.Web.Mvc.dll +namespace ASP +{ + using System; + using System.IO; + using System.Net; + using System.Web; + using System.Web.UI; + using System.Web.WebPages; + + public class _Page_Views_Home_Contact_cshtml : System.Web.Mvc.WebViewPage + { + public _Page_Views_Home_Contact_cshtml() + { + } + + public override void Execute() + { + Layout = "~/_SiteLayout.cshtml"; + Page.Title = "Contact"; + var sayHi = Request.QueryString["sayHi"]; + if (sayHi.IsEmpty()) + { + WriteLiteral(""); // GOOD: hard-coded, not user input + } + else + { + WriteLiteral(sayHi); // BAD: user input flows to HTML unencoded + WriteLiteral(HttpUtility.HtmlEncode(sayHi)); // Good: user input is encoded before it flows to HTML + } + + if (sayHi.IsEmpty()) + { + WriteLiteralTo(Output, ""); // GOOD: hard-coded, not user input + } + else + { + WriteLiteralTo(Output, sayHi); // BAD: user input flows to HTML unencoded + WriteLiteralTo(Output, Html.Encode(sayHi)); // Good: user input is encoded before it flows to HTML + } + + BeginContext("~/Views/Home/Contact.cshtml", 288, 32, false); + + Write(Html.Raw("")); // GOOD: hard-coded, not user input + Write(Html.Raw(Request.QueryString["sayHi"])); // BAD: user input flows to HTML unencoded + Write(Html.Raw(HttpContext.Current.Server.HtmlEncode(Request.QueryString["sayHi"]))); // Good: user input is encoded before it flows to HTML + EndContext("~/Views/Home/Contact.cshtml", 288, 32, false); + } + } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNet/XSS.expected b/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNet/XSS.expected new file mode 100644 index 000000000000..ab165784bffa --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNet/XSS.expected @@ -0,0 +1,3 @@ +| XSS.cs:28:30:28:34 | access to local variable sayHi | $@ flows to here and is written to HTML or JavaScript: System.Web.WebPages.WebPage.WriteLiteral() method. | XSS.cs:21:25:21:43 | access to property QueryString | User-provided value | +| XSS.cs:38:40:38:44 | access to local variable sayHi | $@ flows to here and is written to HTML or JavaScript: System.Web.WebPages.WebPage.WriteLiteralTo() method. | XSS.cs:21:25:21:43 | access to property QueryString | User-provided value | +| XSS.cs:45:28:45:55 | access to indexer | $@ flows to here and is written to HTML or JavaScript. | XSS.cs:45:28:45:46 | access to property QueryString | User-provided value | diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNet/XSS.qlref b/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNet/XSS.qlref new file mode 100644 index 000000000000..faad1d6403c1 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNet/XSS.qlref @@ -0,0 +1 @@ +Security Features/CWE-079/XSS.ql \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNetCore/XSS.cs b/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNetCore/XSS.cs new file mode 100644 index 000000000000..39230b1ca868 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNetCore/XSS.cs @@ -0,0 +1,79 @@ +// semmle-extractor-options: /r:netstandard.dll /r:${testdir}/../../../../../packages/Microsoft.AspNetCore.Mvc.1.1.3/lib/net451/Microsoft.AspNetCore.Mvc.dll /r:${testdir}/../../../../../packages/Microsoft.AspNetCore.Mvc.Core.1.1.3/lib/net451/Microsoft.AspNetCore.Mvc.Core.dll /r:${testdir}/../../../../../packages/Microsoft.AspNetCore.Antiforgery.1.1.2/lib/net451/Microsoft.AspNetCore.Antiforgery.dll /r:${testdir}/../../../../../packages/Microsoft.AspNetCore.Mvc.ViewFeatures.1.1.3/lib/net451/Microsoft.AspNetCore.Mvc.ViewFeatures.dll /r:${testdir}/../../../../../packages/Microsoft.AspNetCore.Mvc.Abstractions.1.1.3/lib/net451/Microsoft.AspNetCore.Mvc.Abstractions.dll /r:${testdir}/../../../../../packages\Microsoft.AspNetCore.Http.Abstractions.1.1.2\lib\net451\Microsoft.AspNetCore.Http.Abstractions.dll /r:${testdir}/../../../../../packages/Microsoft.AspNetCore.Html.Abstractions.1.1.2/lib/netstandard1.0/Microsoft.AspNetCore.Html.Abstractions.dll /r:${testdir}/../../../../../packages/Microsoft.AspNetCore.Http.Features.1.1.2\lib\net451\Microsoft.AspNetCore.Http.Features.dll /r:${testdir}/../../../../../packages\Microsoft.Extensions.Primitives.2.1.0\lib\netstandard2.0\Microsoft.Extensions.Primitives.dll +using System.Linq; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Html; +using Microsoft.Extensions.Primitives; + +namespace Testing.Controllers +{ + public class HomeViewModel + { + public string Message { get; set; } + + public HtmlString Description { get; set; } + } + + public class TestController : Controller + { + public IActionResult Index() + { + // BAD: flow of content type to + var v = new ViewResult(); + v.ViewData["BadData"] = new HtmlString(Request.Query["Bad data"]); + + StringValues vOut; + Request.Query.TryGetValue("Foo", out vOut); + + // BAD: via Enumerable + v.ViewData["FooFirst"] = new HtmlString(vOut.First()); + + // BAD: via toArray + v.ViewData["FooArray0"] = new HtmlString(vOut.ToArray()[0]); + + // BAD: via implicit conversion operator + v.ViewData["FooImplicit"] = new HtmlString(vOut); + + return v; + } + + [HttpPost("Test")] + [ValidateAntiForgeryToken] + public IActionResult Submit([FromQuery] string foo) + { + var view = new ViewResult(); + //BAD: flow of submitted value to view in HtmlString. + view.ViewData["FOO"] = new HtmlString(foo); + return view; + } + + + public IActionResult IndexToModel() + { + //BAD: flow of submitted value to view in HtmlString. + HtmlString v = new HtmlString(Request.QueryString.Value); + return View(new HomeViewModel() { Message = "Message from Index", Description = v }); + } + + public IActionResult About() + { + //BAD: flow of submitted value to view in HtmlString. + HtmlString v = new HtmlString(Request.Query["Foo"].ToString()); + + //BAD: flow of submitted value to view in HtmlString. + HtmlString v1 = new HtmlString(Request.Query["Foo"][0]); + + return View(new HomeViewModel() { Message = "Message from About", Description = v }); + } + + public IActionResult Contact() + { + //BAD: flow of user content type to view in HtmlString. + HtmlString v = new HtmlString(Request.ContentType); + + //BAD: flow of headers to view in HtmlString. + HtmlString v1 = new HtmlString(value: Request.Headers["Foo"]); + + return View(new HomeViewModel() { Message = "Message from Contact", Description = v }); + } + } +} \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNetCore/XSS.expected b/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNetCore/XSS.expected new file mode 100644 index 000000000000..3614a08046b2 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNetCore/XSS.expected @@ -0,0 +1,7 @@ +| XSS.cs:22:52:22:76 | call to operator implicit conversion | $@ flows to here and is written to HTML or JavaScript. | XSS.cs:22:52:22:64 | access to property Query | User-provided value | +| XSS.cs:45:51:45:53 | access to parameter foo | $@ flows to here and is written to HTML or JavaScript. | XSS.cs:41:56:41:58 | foo | User-provided value | +| XSS.cs:53:43:53:67 | access to property Value | $@ flows to here and is written to HTML or JavaScript. | XSS.cs:53:43:53:67 | access to property Value | User-provided value | +| XSS.cs:60:43:60:73 | call to method ToString | $@ flows to here and is written to HTML or JavaScript. | XSS.cs:60:43:60:55 | access to property Query | User-provided value | +| XSS.cs:63:44:63:66 | access to indexer | $@ flows to here and is written to HTML or JavaScript. | XSS.cs:63:44:63:56 | access to property Query | User-provided value | +| XSS.cs:71:43:71:61 | access to property ContentType | $@ flows to here and is written to HTML or JavaScript. | XSS.cs:71:43:71:61 | access to property ContentType | User-provided value | +| XSS.cs:74:51:74:72 | call to operator implicit conversion | $@ flows to here and is written to HTML or JavaScript. | XSS.cs:74:51:74:65 | access to property Headers | User-provided value | diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNetCore/XSS.qlref b/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNetCore/XSS.qlref new file mode 100644 index 000000000000..faad1d6403c1 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSSFlowAspNetCore/XSS.qlref @@ -0,0 +1 @@ +Security Features/CWE-079/XSS.ql \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNETCore/UrlRedirect.cs b/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNETCore/UrlRedirect.cs new file mode 100644 index 000000000000..f593ad66f9d1 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNETCore/UrlRedirect.cs @@ -0,0 +1,65 @@ +// semmle-extractor-options: /r:netstandard.dll /r:${testdir}/../../../../packages\Microsoft.AspNetCore.Mvc.2.1.0\lib\netstandard2.0\Microsoft.AspNetCore.Mvc.dll /r:${testdir}/../../../../packages\Microsoft.AspNetCore.Mvc.Core.2.1.0\lib\netstandard2.0\Microsoft.AspNetCore.Mvc.Core.dll /r:${testdir}/../../../../packages\Microsoft.AspNetCore.Http.Extensions.2.1.0\lib\netstandard2.0\Microsoft.AspNetCore.Http.Extensions.dll /r:${testdir}/../../../../packages\Microsoft.AspNetCore.Http.Abstractions.2.1.0\lib\netstandard2.0\Microsoft.AspNetCore.Http.Abstractions.dll /r:${testdir}/../../../../packages\Microsoft.AspNetCore.Mvc.Abstractions.2.1.0\lib\netstandard2.0\Microsoft.AspNetCore.Mvc.Abstractions.dll /r:${testdir}/../../../../packages\Microsoft.AspNetCore.Http.Features.2.1.0\lib\netstandard2.0\Microsoft.AspNetCore.Http.Features.dll /r:${testdir}/../../../../packages\Microsoft.Extensions.Primitives.2.1.0\lib\netstandard2.0\Microsoft.Extensions.Primitives.dll + +using System; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Headers; +using Microsoft.AspNetCore.Mvc; + +namespace Testing.Controllers +{ + public class SomeController : ControllerBase + { + private static string SomeValue = "HeaderValue"; + + [HttpPost] + public void Post([FromBody] string value) + { + // BAD: straight up controller redirect + Redirect(value); + + // BAD: Setting response headers collection, location = redirect + Response.Headers["location"] = value; + + // GOOD: Setting response header to a constant value + Response.Headers["location"] = SomeValue; + + // BAD: Setting response headers collection, location = redirect via add method + Response.Headers.Add("location", value); + + // GOOD: Setting response header to a constant value + Response.Headers.Add("location", "foo"); + + // BAD: redirect via location + Response.Headers.SetCommaSeparatedValues("location", value); + + // BAD = redirect via setting location value from tainted source + Response.Headers.Append("location", value); + + // BAD: redirect via setting location header from comma-separated values + Response.Headers.AppendCommaSeparatedValues("location", value); + + // BAD: tainted redirect to Action + RedirectToActionPermanent("Error" + value); + } + + // PUT: api/Some/5 + [HttpPut("{id}")] + public void Put(int id, [FromBody] string value) + { + + RedirectToPage(value); + + var headers = new ResponseHeaders(Response.Headers); + + // BAD: redirect via header helper class + headers.Location = new Uri(value); + + // BAD: response redirect + Response.Redirect(value); + + // GOOD: whitelisted redirect + if(Url.IsLocalUrl(value)) + Redirect(value); + } + } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNETCore/UrlRedirect.expected b/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNETCore/UrlRedirect.expected new file mode 100644 index 000000000000..819fc89c6c61 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNETCore/UrlRedirect.expected @@ -0,0 +1,10 @@ +| TestController.cs:18:22:18:26 | access to parameter value | Untrusted URL redirection due to $@. | UrlRedirect.cs:15:44:15:48 | value | user-provided value | +| UrlRedirect.cs:21:44:21:48 | call to operator implicit conversion | Untrusted URL redirection due to $@. | UrlRedirect.cs:15:44:15:48 | value | user-provided value | +| UrlRedirect.cs:27:46:27:50 | call to operator implicit conversion | Untrusted URL redirection due to $@. | UrlRedirect.cs:15:44:15:48 | value | user-provided value | +| UrlRedirect.cs:33:66:33:70 | access to parameter value | Untrusted URL redirection due to $@. | UrlRedirect.cs:15:44:15:48 | value | user-provided value | +| UrlRedirect.cs:36:49:36:53 | call to operator implicit conversion | Untrusted URL redirection due to $@. | UrlRedirect.cs:15:44:15:48 | value | user-provided value | +| UrlRedirect.cs:39:69:39:73 | access to parameter value | Untrusted URL redirection due to $@. | UrlRedirect.cs:15:44:15:48 | value | user-provided value | +| UrlRedirect.cs:42:39:42:53 | ... + ... | Untrusted URL redirection due to $@. | UrlRedirect.cs:15:44:15:48 | value | user-provided value | +| UrlRedirect.cs:50:28:50:32 | access to parameter value | Untrusted URL redirection due to $@. | UrlRedirect.cs:47:51:47:55 | value | user-provided value | +| UrlRedirect.cs:55:32:55:45 | object creation of type Uri | Untrusted URL redirection due to $@. | UrlRedirect.cs:47:51:47:55 | value | user-provided value | +| UrlRedirect.cs:58:31:58:35 | access to parameter value | Untrusted URL redirection due to $@. | UrlRedirect.cs:47:51:47:55 | value | user-provided value | diff --git a/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.qlref b/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNETCore/UrlRedirect.qlref similarity index 100% rename from csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.qlref rename to csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNETCore/UrlRedirect.qlref diff --git a/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.cs b/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNetClassic/UrlRedirect.cs similarity index 95% rename from csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.cs rename to csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNetClassic/UrlRedirect.cs index 4ced49db6a83..8378e6743d90 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.cs +++ b/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNetClassic/UrlRedirect.cs @@ -1,4 +1,4 @@ -// semmle-extractor-options: ${testdir}/../../../resources/stubs/System.Web.cs /r:System.Collections.Specialized.dll +// semmle-extractor-options: ${testdir}/../../../../resources/stubs/System.Web.cs /r:System.Collections.Specialized.dll using System; using System.Web; diff --git a/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.expected b/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNetClassic/UrlRedirect.expected similarity index 100% rename from csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.expected rename to csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNetClassic/UrlRedirect.expected diff --git a/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNetClassic/UrlRedirect.qlref b/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNetClassic/UrlRedirect.qlref new file mode 100644 index 000000000000..2e061145f9ca --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect.ASPNetClassic/UrlRedirect.qlref @@ -0,0 +1 @@ +Security Features/CWE-601/UrlRedirect.ql