diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll index 2906fde4e1de..2e2cbbe79c97 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll @@ -3,6 +3,7 @@ */ import csharp +private import semmle.code.csharp.commons.Collections private import semmle.code.csharp.frameworks.system.Net private import semmle.code.csharp.frameworks.system.Web private import semmle.code.csharp.frameworks.system.web.Http @@ -104,7 +105,7 @@ class WcfRemoteFlowSource extends RemoteFlowSource, DataFlow::ParameterNode { } /** A data flow source of remote user input (ASP.NET web service). */ -class AspNetServiceRemoteFlowSource extends RemoteFlowSource, DataFlow::ParameterNode { +class AspNetServiceRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::ParameterNode { AspNetServiceRemoteFlowSource() { exists(Method m | m.getAParameter() = this.getParameter() and @@ -115,8 +116,44 @@ class AspNetServiceRemoteFlowSource extends RemoteFlowSource, DataFlow::Paramete override string getSourceType() { result = "ASP.NET web service input" } } +/** + * Taint members (transitively) on types used in + * 1. Action method parameters. + * 2. WebMethod parameters. + * + * Note, as this also impact uses of such types in other contexts, we only do this for + * user-defined types. + */ +private class AspNetRemoteFlowSourceMember extends TaintTracking::TaintedMember { + AspNetRemoteFlowSourceMember() { + exists(Type t, Type t0 | t = this.getDeclaringType() and t.fromSource() | + (t = t0 or t = t0.(ArrayType).getElementType()) and + ( + t0 = any(AspNetRemoteFlowSourceMember m).getType() + or + t0 = any(ActionMethodParameter p).getType() + or + t0 = any(AspNetServiceRemoteFlowSource source).getType() + ) + ) and + this.isPublic() and + not this.isStatic() and + ( + this = + any(Property p | + p.isAutoImplemented() and + p.getGetter().isPublic() and + p.getSetter().isPublic() + ) + or + this = any(Field f | f.isPublic()) + ) + } +} + /** A data flow source of remote user input (ASP.NET request message). */ -class SystemNetHttpRequestMessageRemoteFlowSource extends RemoteFlowSource, DataFlow::ExprNode { +class SystemNetHttpRequestMessageRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::ExprNode +{ SystemNetHttpRequestMessageRemoteFlowSource() { this.getType() instanceof SystemWebHttpRequestMessageClass } @@ -166,7 +203,7 @@ class MicrosoftOwinRequestRemoteFlowSource extends RemoteFlowSource, DataFlow::E } /** A parameter to an Mvc controller action method, viewed as a source of remote user input. */ -class ActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode { +class ActionMethodParameter extends AspNetRemoteFlowSource, DataFlow::ParameterNode { ActionMethodParameter() { exists(Parameter p | p = this.getParameter() and diff --git a/csharp/ql/test/library-tests/dataflow/flowsources/remote/RemoteFlowSource.cs b/csharp/ql/test/library-tests/dataflow/flowsources/remote/RemoteFlowSource.cs index 5889183f5257..3c7cbcba04a2 100644 --- a/csharp/ql/test/library-tests/dataflow/flowsources/remote/RemoteFlowSource.cs +++ b/csharp/ql/test/library-tests/dataflow/flowsources/remote/RemoteFlowSource.cs @@ -54,3 +54,51 @@ public static async void M3(System.Net.WebSockets.WebSocket webSocket) } } } + +namespace AspRemoteFlowSource +{ + using System.Web.Services; + + public class MySubData + { + public string SubDataProp { get; set; } + } + + public class MyElementSubData + { + public string ElementSubDataProp { get; set; } + } + + public class MyData + { + public string DataField; + public string DataProp { get; set; } + public MySubData SubData { get; set; } + public MyElementSubData[] Elements { get; set; } + } + + public class MyDataElement + { + public string Prop { get; set; } + } + + + public class MyService + { + [WebMethod] + public void MyMethod(MyData data) + { + Use(data.DataProp); + Use(data.SubData.SubDataProp); + Use(data.Elements[0].ElementSubDataProp); + } + + [WebMethod] + public void MyMethod2(MyDataElement[] data) + { + Use(data[0].Prop); + } + + public static void Use(object o) { } + } +} diff --git a/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected b/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected index f5f541d73d53..ef70ca9ad93a 100644 --- a/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected +++ b/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected @@ -1,3 +1,13 @@ +remoteFlowSourceMembers +| Controller.cs:6:19:6:25 | Tainted | +| RemoteFlowSource.cs:64:23:64:33 | SubDataProp | +| RemoteFlowSource.cs:69:23:69:40 | ElementSubDataProp | +| RemoteFlowSource.cs:74:23:74:31 | DataField | +| RemoteFlowSource.cs:75:23:75:30 | DataProp | +| RemoteFlowSource.cs:76:26:76:32 | SubData | +| RemoteFlowSource.cs:77:35:77:42 | Elements | +| RemoteFlowSource.cs:82:23:82:26 | Prop | +remoteFlowSources | Controller.cs:11:43:11:52 | sampleData | ASP.NET MVC action method parameter | | Controller.cs:11:62:11:66 | taint | ASP.NET MVC action method parameter | | Controller.cs:16:43:16:52 | sampleData | ASP.NET MVC action method parameter | @@ -10,3 +20,5 @@ | RemoteFlowSource.cs:45:17:45:23 | access to parameter request | ASP.NET query string | | RemoteFlowSource.cs:45:17:45:42 | access to property RawUrl | ASP.NET unvalidated request data | | RemoteFlowSource.cs:52:55:52:61 | [post] access to local variable segment | external | +| RemoteFlowSource.cs:89:37:89:40 | data | ASP.NET web service input | +| RemoteFlowSource.cs:97:47:97:50 | data | ASP.NET web service input | diff --git a/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql b/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql index fdea5323d5cb..f6d87eb9ff4f 100644 --- a/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql +++ b/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql @@ -1,5 +1,7 @@ import semmle.code.csharp.security.dataflow.flowsources.Remote -from RemoteFlowSource source -where source.getLocation().getFile().fromSource() -select source, source.getSourceType() +query predicate remoteFlowSourceMembers(TaintTracking::TaintedMember m) { m.fromSource() } + +query predicate remoteFlowSources(RemoteFlowSource source, string type) { + source.getLocation().getFile().fromSource() and type = source.getSourceType() +} diff --git a/csharp/ql/test/resources/stubs/System.Web.cs b/csharp/ql/test/resources/stubs/System.Web.cs index c15b871095ff..23ae0f298cf4 100644 --- a/csharp/ql/test/resources/stubs/System.Web.cs +++ b/csharp/ql/test/resources/stubs/System.Web.cs @@ -454,3 +454,8 @@ public class SimpleTypeResolver : System.Web.Script.Serialization.JavaScriptType public SimpleTypeResolver() => throw null; } } + +namespace System.Web.Services +{ + public class WebMethodAttribute : Attribute { } +}