From 00de19ce13b52f9af11bef5407c1ad344439699a Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 8 Nov 2024 09:41:45 +0100 Subject: [PATCH 1/3] C#: Deprecate experimental queries and libraries. --- .../experimental/CWE-099/TaintedWebClient.ql | 18 ++- .../CWE-099/TaintedWebClientLib.qll | 2 + .../experimental/CWE-918/RequestForgery.ql | 18 ++- .../experimental/CWE-918/RequestForgery.qll | 2 + .../CWE-1004/CookieWithoutHttpOnly.ql | 150 +++++++++--------- ...nsafeUsageOfClientSideEncryptionVersion.ql | 11 +- .../CWE-614/CookieWithoutSecure.ql | 148 ++++++++--------- .../CWE-759/HashWithoutSalt.ql | 14 +- .../JsonWebTokenHandlerLib.qll | 2 + ...security-validations-always-return-true.ql | 18 ++- .../security-validation-disabled.ql | 19 ++- .../Serialization/DataSetSerialization.qll | 1 + .../DefiningDatasetRelatedType.ql | 11 +- .../DefiningPotentiallyUnsafeXmlSerializer.ql | 19 ++- .../UnsafeTypeUsedDataContractSerializer.ql | 22 +-- .../XmlDeserializationWithDataSet.ql | 10 +- .../backdoor/DangerousNativeFunctionCall.ql | 8 +- .../backdoor/PotentialTimeBomb.ql | 21 +-- .../backdoor/ProcessNameToHashTaintFlow.ql | 17 +- .../dataflow/flowsources/AuthCookie.qll | 1 + 20 files changed, 286 insertions(+), 226 deletions(-) diff --git a/csharp/ql/src/experimental/CWE-099/TaintedWebClient.ql b/csharp/ql/src/experimental/CWE-099/TaintedWebClient.ql index 38d99db65900..338036155441 100644 --- a/csharp/ql/src/experimental/CWE-099/TaintedWebClient.ql +++ b/csharp/ql/src/experimental/CWE-099/TaintedWebClient.ql @@ -15,10 +15,16 @@ */ import csharp -import TaintedWebClientLib -import TaintedWebClient::PathGraph +deprecated import TaintedWebClientLib +deprecated import TaintedWebClient::PathGraph -from TaintedWebClient::PathNode source, TaintedWebClient::PathNode sink -where TaintedWebClient::flowPath(source, sink) -select sink.getNode(), source, sink, "A method of WebClient depepends on a $@.", source.getNode(), - "user-provided value" +deprecated query predicate problems( + DataFlow::Node sinkNode, TaintedWebClient::PathNode source, TaintedWebClient::PathNode sink, + string message1, DataFlow::Node sourceNode, string message2 +) { + TaintedWebClient::flowPath(source, sink) and + sinkNode = sink.getNode() and + message1 = "A method of WebClient depepends on a $@." and + sourceNode = source.getNode() and + message2 = "user-provided value" +} diff --git a/csharp/ql/src/experimental/CWE-099/TaintedWebClientLib.qll b/csharp/ql/src/experimental/CWE-099/TaintedWebClientLib.qll index 716702ca008c..a088f5100af8 100644 --- a/csharp/ql/src/experimental/CWE-099/TaintedWebClientLib.qll +++ b/csharp/ql/src/experimental/CWE-099/TaintedWebClientLib.qll @@ -1,3 +1,5 @@ +deprecated module; + import csharp import semmle.code.csharp.frameworks.system.Net import semmle.code.csharp.frameworks.System diff --git a/csharp/ql/src/experimental/CWE-918/RequestForgery.ql b/csharp/ql/src/experimental/CWE-918/RequestForgery.ql index 9f59da7e38df..313e3c76c0e8 100644 --- a/csharp/ql/src/experimental/CWE-918/RequestForgery.ql +++ b/csharp/ql/src/experimental/CWE-918/RequestForgery.ql @@ -11,10 +11,16 @@ */ import csharp -import RequestForgery::RequestForgery -import RequestForgeryFlow::PathGraph +deprecated import RequestForgery::RequestForgery +deprecated import RequestForgeryFlow::PathGraph -from RequestForgeryFlow::PathNode source, RequestForgeryFlow::PathNode sink -where RequestForgeryFlow::flowPath(source, sink) -select sink.getNode(), source, sink, "The URL of this request depends on a $@.", source.getNode(), - "user-provided value" +deprecated query predicate problems( + DataFlow::Node sinkNode, RequestForgeryFlow::PathNode source, RequestForgeryFlow::PathNode sink, + string message1, DataFlow::Node sourceNode, string message2 +) { + RequestForgeryFlow::flowPath(source, sink) and + sinkNode = sink.getNode() and + message1 = "The URL of this request depends on a $@." and + sourceNode = source.getNode() and + message2 = "user-provided value" +} diff --git a/csharp/ql/src/experimental/CWE-918/RequestForgery.qll b/csharp/ql/src/experimental/CWE-918/RequestForgery.qll index 6d06ca5fa445..9ab1351f4142 100644 --- a/csharp/ql/src/experimental/CWE-918/RequestForgery.qll +++ b/csharp/ql/src/experimental/CWE-918/RequestForgery.qll @@ -1,3 +1,5 @@ +deprecated module; + import csharp module RequestForgery { diff --git a/csharp/ql/src/experimental/Security Features/CWE-1004/CookieWithoutHttpOnly.ql b/csharp/ql/src/experimental/Security Features/CWE-1004/CookieWithoutHttpOnly.ql index 560dcc6dd74a..359ffbcd2f30 100644 --- a/csharp/ql/src/experimental/Security Features/CWE-1004/CookieWithoutHttpOnly.ql +++ b/csharp/ql/src/experimental/Security Features/CWE-1004/CookieWithoutHttpOnly.ql @@ -17,89 +17,91 @@ import csharp import semmle.code.asp.WebConfig import semmle.code.csharp.frameworks.system.Web import semmle.code.csharp.frameworks.microsoft.AspNetCore -import experimental.dataflow.flowsources.AuthCookie +deprecated import experimental.dataflow.flowsources.AuthCookie -from Expr httpOnlySink -where - exists(Assignment a, Expr val | - httpOnlySink = a.getRValue() and - val.getValue() = "false" and - ( - exists(ObjectCreation oc | - getAValueForProp(oc, a, "HttpOnly") = val and - ( - oc.getType() instanceof SystemWebHttpCookie and - isCookieWithSensitiveName(oc.getArgument(0)) - or - exists(MethodCall mc, MicrosoftAspNetCoreHttpResponseCookies iResponse | - oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and - iResponse.getAppendMethod() = mc.getTarget() and - isCookieWithSensitiveName(mc.getArgument(0)) and - // there is no callback `OnAppendCookie` that sets `HttpOnly` to true - not OnAppendCookieHttpOnlyTracking::flowTo(_) and - // Passed as third argument to `IResponseCookies.Append` - exists(DataFlow::Node creation, DataFlow::Node append | - CookieOptionsTracking::flow(creation, append) and - creation.asExpr() = oc and - append.asExpr() = mc.getArgument(2) +deprecated query predicate problems(Expr httpOnlySink, string message) { + ( + exists(Assignment a, Expr val | + httpOnlySink = a.getRValue() and + val.getValue() = "false" and + ( + exists(ObjectCreation oc | + getAValueForProp(oc, a, "HttpOnly") = val and + ( + oc.getType() instanceof SystemWebHttpCookie and + isCookieWithSensitiveName(oc.getArgument(0)) + or + exists(MethodCall mc, MicrosoftAspNetCoreHttpResponseCookies iResponse | + oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and + iResponse.getAppendMethod() = mc.getTarget() and + isCookieWithSensitiveName(mc.getArgument(0)) and + // there is no callback `OnAppendCookie` that sets `HttpOnly` to true + not OnAppendCookieHttpOnlyTracking::flowTo(_) and + // Passed as third argument to `IResponseCookies.Append` + exists(DataFlow::Node creation, DataFlow::Node append | + CookieOptionsTracking::flow(creation, append) and + creation.asExpr() = oc and + append.asExpr() = mc.getArgument(2) + ) ) ) ) - ) - or - exists(PropertyWrite pw | - ( - pw.getProperty().getDeclaringType() instanceof MicrosoftAspNetCoreHttpCookieBuilder or - pw.getProperty().getDeclaringType() instanceof - MicrosoftAspNetCoreAuthenticationCookiesCookieAuthenticationOptions - ) and - pw.getProperty().getName() = "HttpOnly" and - a.getLValue() = pw and - DataFlow::localExprFlow(val, a.getRValue()) + or + exists(PropertyWrite pw | + ( + pw.getProperty().getDeclaringType() instanceof MicrosoftAspNetCoreHttpCookieBuilder or + pw.getProperty().getDeclaringType() instanceof + MicrosoftAspNetCoreAuthenticationCookiesCookieAuthenticationOptions + ) and + pw.getProperty().getName() = "HttpOnly" and + a.getLValue() = pw and + DataFlow::localExprFlow(val, a.getRValue()) + ) ) ) - ) - or - exists(Call c | - httpOnlySink = c and - ( - exists(MicrosoftAspNetCoreHttpResponseCookies iResponse, MethodCall mc | - // default is not configured or is not set to `Always` - not getAValueForCookiePolicyProp("HttpOnly").getValue() = "1" and - // there is no callback `OnAppendCookie` that sets `HttpOnly` to true - not OnAppendCookieHttpOnlyTracking::flowTo(_) and - iResponse.getAppendMethod() = mc.getTarget() and - isCookieWithSensitiveName(mc.getArgument(0)) and - ( - // `HttpOnly` property in `CookieOptions` passed to IResponseCookies.Append(...) wasn't set - exists(ObjectCreation oc | - oc = c and - oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and - not isPropertySet(oc, "HttpOnly") and - exists(DataFlow::Node creation | - CookieOptionsTracking::flow(creation, _) and - creation.asExpr() = oc + or + exists(Call c | + httpOnlySink = c and + ( + exists(MicrosoftAspNetCoreHttpResponseCookies iResponse, MethodCall mc | + // default is not configured or is not set to `Always` + not getAValueForCookiePolicyProp("HttpOnly").getValue() = "1" and + // there is no callback `OnAppendCookie` that sets `HttpOnly` to true + not OnAppendCookieHttpOnlyTracking::flowTo(_) and + iResponse.getAppendMethod() = mc.getTarget() and + isCookieWithSensitiveName(mc.getArgument(0)) and + ( + // `HttpOnly` property in `CookieOptions` passed to IResponseCookies.Append(...) wasn't set + exists(ObjectCreation oc | + oc = c and + oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and + not isPropertySet(oc, "HttpOnly") and + exists(DataFlow::Node creation | + CookieOptionsTracking::flow(creation, _) and + creation.asExpr() = oc + ) ) + or + // IResponseCookies.Append(String, String) was called, `HttpOnly` is set to `false` by default + mc = c and + mc.getNumberOfArguments() < 3 ) - or - // IResponseCookies.Append(String, String) was called, `HttpOnly` is set to `false` by default - mc = c and - mc.getNumberOfArguments() < 3 ) - ) - or - exists(ObjectCreation oc | - oc = c and - oc.getType() instanceof SystemWebHttpCookie and - isCookieWithSensitiveName(oc.getArgument(0)) and - // the property wasn't explicitly set, so a default value from config is used - not isPropertySet(oc, "HttpOnly") and - // the default in config is not set to `true` - not exists(XmlElement element | - element instanceof HttpCookiesElement and - element.(HttpCookiesElement).isHttpOnlyCookies() + or + exists(ObjectCreation oc | + oc = c and + oc.getType() instanceof SystemWebHttpCookie and + isCookieWithSensitiveName(oc.getArgument(0)) and + // the property wasn't explicitly set, so a default value from config is used + not isPropertySet(oc, "HttpOnly") and + // the default in config is not set to `true` + not exists(XmlElement element | + element instanceof HttpCookiesElement and + element.(HttpCookiesElement).isHttpOnlyCookies() + ) ) ) ) - ) -select httpOnlySink, "Cookie attribute 'HttpOnly' is not set to true." + ) and + message = "Cookie attribute 'HttpOnly' is not set to true." +} diff --git a/csharp/ql/src/experimental/Security Features/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql b/csharp/ql/src/experimental/Security Features/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql index e112d2848fff..d8bbbce70658 100644 --- a/csharp/ql/src/experimental/Security Features/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql +++ b/csharp/ql/src/experimental/Security Features/CWE-327/Azure/UnsafeUsageOfClientSideEncryptionVersion.ql @@ -68,15 +68,14 @@ predicate isExprAnAccessToSafeClientSideEncryptionVersionValue(Expr e) { ) } -from Expr e, Class c, Assembly asm -where - asm = c.getLocation() and - ( +deprecated query predicate problems(Expr e, string message) { + exists(Class c, Assembly asm | asm = c.getLocation() | exists(Expr e2 | isCreatingAzureClientSideEncryptionObject(e, c, e2) and not isObjectCreationArgumentSafeAndUsingSafeVersionOfAssembly(e2, asm) ) or isCreatingOutdatedAzureClientSideEncryptionObject(e, c) - ) -select e, "Unsafe usage of v1 version of Azure Storage client-side encryption." + ) and + message = "Unsafe usage of v1 version of Azure Storage client-side encryption." +} diff --git a/csharp/ql/src/experimental/Security Features/CWE-614/CookieWithoutSecure.ql b/csharp/ql/src/experimental/Security Features/CWE-614/CookieWithoutSecure.ql index 417c5e592aaf..d7628f7b2c7b 100644 --- a/csharp/ql/src/experimental/Security Features/CWE-614/CookieWithoutSecure.ql +++ b/csharp/ql/src/experimental/Security Features/CWE-614/CookieWithoutSecure.ql @@ -17,89 +17,91 @@ import csharp import semmle.code.asp.WebConfig import semmle.code.csharp.frameworks.system.Web import semmle.code.csharp.frameworks.microsoft.AspNetCore -import experimental.dataflow.flowsources.AuthCookie +deprecated import experimental.dataflow.flowsources.AuthCookie -from Expr secureSink -where - exists(Call c | - secureSink = c and - ( - // default is not configured or is not set to `Always` or `SameAsRequest` - not ( - getAValueForCookiePolicyProp("Secure").getValue() = "0" or - getAValueForCookiePolicyProp("Secure").getValue() = "1" - ) and - // there is no callback `OnAppendCookie` that sets `Secure` to true - not OnAppendCookieSecureTracking::flowTo(_) and +deprecated query predicate problems(Expr secureSink, string message) { + ( + exists(Call c | + secureSink = c and ( - // `Secure` property in `CookieOptions` passed to IResponseCookies.Append(...) wasn't set + // default is not configured or is not set to `Always` or `SameAsRequest` + not ( + getAValueForCookiePolicyProp("Secure").getValue() = "0" or + getAValueForCookiePolicyProp("Secure").getValue() = "1" + ) and + // there is no callback `OnAppendCookie` that sets `Secure` to true + not OnAppendCookieSecureTracking::flowTo(_) and + ( + // `Secure` property in `CookieOptions` passed to IResponseCookies.Append(...) wasn't set + exists(ObjectCreation oc | + oc = c and + oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and + not isPropertySet(oc, "Secure") and + exists(DataFlow::Node creation | + CookieOptionsTracking::flow(creation, _) and + creation.asExpr() = oc + ) + ) + or + // IResponseCookies.Append(String, String) was called, `Secure` is set to `false` by default + exists(MethodCall mc, MicrosoftAspNetCoreHttpResponseCookies iResponse | + mc = c and + iResponse.getAppendMethod() = mc.getTarget() and + mc.getNumberOfArguments() < 3 + ) + ) + or exists(ObjectCreation oc | oc = c and - oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and + oc.getType() instanceof SystemWebHttpCookie and + // the property wasn't explicitly set, so a default value from config is used not isPropertySet(oc, "Secure") and - exists(DataFlow::Node creation | - CookieOptionsTracking::flow(creation, _) and - creation.asExpr() = oc + // the default in config is not set to `true` + // the `exists` below covers the `cs/web/requiressl-not-set` + not exists(XmlElement element | + element instanceof FormsElement and + element.(FormsElement).isRequireSsl() + or + element instanceof HttpCookiesElement and + element.(HttpCookiesElement).isRequireSsl() ) ) - or - // IResponseCookies.Append(String, String) was called, `Secure` is set to `false` by default - exists(MethodCall mc, MicrosoftAspNetCoreHttpResponseCookies iResponse | - mc = c and - iResponse.getAppendMethod() = mc.getTarget() and - mc.getNumberOfArguments() < 3 - ) - ) - or - exists(ObjectCreation oc | - oc = c and - oc.getType() instanceof SystemWebHttpCookie and - // the property wasn't explicitly set, so a default value from config is used - not isPropertySet(oc, "Secure") and - // the default in config is not set to `true` - // the `exists` below covers the `cs/web/requiressl-not-set` - not exists(XmlElement element | - element instanceof FormsElement and - element.(FormsElement).isRequireSsl() - or - element instanceof HttpCookiesElement and - element.(HttpCookiesElement).isRequireSsl() - ) ) ) - ) - or - exists(Assignment a, Expr val | - secureSink = a.getRValue() and - ( - exists(ObjectCreation oc | - getAValueForProp(oc, a, "Secure") = val and - val.getValue() = "false" and - ( - oc.getType() instanceof SystemWebHttpCookie - or - oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and - // there is no callback `OnAppendCookie` that sets `Secure` to true - not OnAppendCookieSecureTracking::flowTo(_) and - // the cookie option is passed to `Append` - exists(DataFlow::Node creation | - CookieOptionsTracking::flow(creation, _) and - creation.asExpr() = oc + or + exists(Assignment a, Expr val | + secureSink = a.getRValue() and + ( + exists(ObjectCreation oc | + getAValueForProp(oc, a, "Secure") = val and + val.getValue() = "false" and + ( + oc.getType() instanceof SystemWebHttpCookie + or + oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and + // there is no callback `OnAppendCookie` that sets `Secure` to true + not OnAppendCookieSecureTracking::flowTo(_) and + // the cookie option is passed to `Append` + exists(DataFlow::Node creation | + CookieOptionsTracking::flow(creation, _) and + creation.asExpr() = oc + ) ) ) - ) - or - exists(PropertyWrite pw | - ( - pw.getProperty().getDeclaringType() instanceof MicrosoftAspNetCoreHttpCookieBuilder or - pw.getProperty().getDeclaringType() instanceof - MicrosoftAspNetCoreAuthenticationCookiesCookieAuthenticationOptions - ) and - pw.getProperty().getName() = "SecurePolicy" and - a.getLValue() = pw and - DataFlow::localExprFlow(val, a.getRValue()) and - val.getValue() = "2" // None + or + exists(PropertyWrite pw | + ( + pw.getProperty().getDeclaringType() instanceof MicrosoftAspNetCoreHttpCookieBuilder or + pw.getProperty().getDeclaringType() instanceof + MicrosoftAspNetCoreAuthenticationCookiesCookieAuthenticationOptions + ) and + pw.getProperty().getName() = "SecurePolicy" and + a.getLValue() = pw and + DataFlow::localExprFlow(val, a.getRValue()) and + val.getValue() = "2" // None + ) ) ) - ) -select secureSink, "Cookie attribute 'Secure' is not set to true." + ) and + message = "Cookie attribute 'Secure' is not set to true." +} diff --git a/csharp/ql/src/experimental/Security Features/CWE-759/HashWithoutSalt.ql b/csharp/ql/src/experimental/Security Features/CWE-759/HashWithoutSalt.ql index f9c279e09bf0..f18798c8b086 100644 --- a/csharp/ql/src/experimental/Security Features/CWE-759/HashWithoutSalt.ql +++ b/csharp/ql/src/experimental/Security Features/CWE-759/HashWithoutSalt.ql @@ -192,7 +192,13 @@ module HashWithoutSaltConfig implements DataFlow::ConfigSig { module HashWithoutSalt = TaintTracking::Global; -from HashWithoutSalt::PathNode source, HashWithoutSalt::PathNode sink -where HashWithoutSalt::flowPath(source, sink) -select sink.getNode(), source, sink, "$@ is hashed without a salt.", source.getNode(), - "The password" +deprecated query predicate problems( + DataFlow::Node sinkNode, HashWithoutSalt::PathNode source, HashWithoutSalt::PathNode sink, + string message, DataFlow::Node sourceNode, string password +) { + sinkNode = sink.getNode() and + sourceNode = source.getNode() and + HashWithoutSalt::flowPath(source, sink) and + message = "$@ is hashed without a salt." and + password = "The password" +} diff --git a/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/JsonWebTokenHandlerLib.qll b/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/JsonWebTokenHandlerLib.qll index 476b17e4c695..ae2c1442e7c2 100644 --- a/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/JsonWebTokenHandlerLib.qll +++ b/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/JsonWebTokenHandlerLib.qll @@ -1,3 +1,5 @@ +deprecated module; + import csharp import DataFlow diff --git a/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/delegated-security-validations-always-return-true.ql b/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/delegated-security-validations-always-return-true.ql index 39aaa80935a7..0804f91d54e8 100644 --- a/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/delegated-security-validations-always-return-true.ql +++ b/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/delegated-security-validations-always-return-true.ql @@ -14,11 +14,17 @@ import csharp import DataFlow -import JsonWebTokenHandlerLib +deprecated import JsonWebTokenHandlerLib import semmle.code.csharp.commons.QualifiedName -from TokenValidationParametersProperty p, CallableAlwaysReturnsTrue e, string qualifier, string name -where e = p.getAnAssignedValue() and p.hasFullyQualifiedName(qualifier, name) -select e, - "JsonWebTokenHandler security-sensitive property $@ is being delegated to this callable that always returns \"true\".", - p, getQualifiedName(qualifier, name) +deprecated query predicate problems( + CallableAlwaysReturnsTrue e, string message, TokenValidationParametersProperty p, + string fullyQualifiedName +) { + exists(string qualifier, string name | p.hasFullyQualifiedName(qualifier, name) | + fullyQualifiedName = getQualifiedName(qualifier, name) + ) and + e = p.getAnAssignedValue() and + message = + "JsonWebTokenHandler security-sensitive property $@ is being delegated to this callable that always returns \"true\"." +} diff --git a/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled.ql b/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled.ql index 679e6c3bcfc9..ab4afe966963 100644 --- a/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled.ql +++ b/csharp/ql/src/experimental/Security Features/JsonWebTokenHandler/security-validation-disabled.ql @@ -12,15 +12,18 @@ */ import csharp -import JsonWebTokenHandlerLib +deprecated import JsonWebTokenHandlerLib import semmle.code.csharp.commons.QualifiedName -from - DataFlow::Node source, DataFlow::Node sink, - TokenValidationParametersPropertySensitiveValidation pw, string qualifier, string name -where +deprecated query predicate problems( + DataFlow::Node sink, string message, TokenValidationParametersPropertySensitiveValidation pw, + string fullyQualifiedName, DataFlow::Node source, string value +) { FalseValueFlowsToTokenValidationParametersPropertyWriteToBypassValidation::flow(source, sink) and sink.asExpr() = pw.getAnAssignedValue() and - pw.hasFullyQualifiedName(qualifier, name) -select sink, "The security sensitive property $@ is being disabled by the following value: $@.", pw, - getQualifiedName(qualifier, name), source, "false" + exists(string qualifier, string name | pw.hasFullyQualifiedName(qualifier, name) | + fullyQualifiedName = getQualifiedName(qualifier, name) + ) and + message = "The security sensitive property $@ is being disabled by the following value: $@." and + value = "false" +} diff --git a/csharp/ql/src/experimental/Security Features/Serialization/DataSetSerialization.qll b/csharp/ql/src/experimental/Security Features/Serialization/DataSetSerialization.qll index ca5086177215..324dc883a352 100644 --- a/csharp/ql/src/experimental/Security Features/Serialization/DataSetSerialization.qll +++ b/csharp/ql/src/experimental/Security Features/Serialization/DataSetSerialization.qll @@ -3,6 +3,7 @@ * * Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details. */ +deprecated module; import csharp diff --git a/csharp/ql/src/experimental/Security Features/Serialization/DefiningDatasetRelatedType.ql b/csharp/ql/src/experimental/Security Features/Serialization/DefiningDatasetRelatedType.ql index 47153f926c1b..2e62f406c1a4 100644 --- a/csharp/ql/src/experimental/Security Features/Serialization/DefiningDatasetRelatedType.ql +++ b/csharp/ql/src/experimental/Security Features/Serialization/DefiningDatasetRelatedType.ql @@ -9,9 +9,10 @@ */ import csharp -import DataSetSerialization +deprecated import DataSetSerialization -from DataSetOrTableRelatedClass dstc -where dstc.fromSource() -select dstc, - "Defining a class that inherits or has a property derived from the obsolete DataSet or DataTable types. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details." +deprecated query predicate problems(DataSetOrTableRelatedClass dstc, string message) { + dstc.fromSource() and + message = + "Defining a class that inherits or has a property derived from the obsolete DataSet or DataTable types. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details." +} diff --git a/csharp/ql/src/experimental/Security Features/Serialization/DefiningPotentiallyUnsafeXmlSerializer.ql b/csharp/ql/src/experimental/Security Features/Serialization/DefiningPotentiallyUnsafeXmlSerializer.ql index 0e87f724b962..7b8631eac395 100644 --- a/csharp/ql/src/experimental/Security Features/Serialization/DefiningPotentiallyUnsafeXmlSerializer.ql +++ b/csharp/ql/src/experimental/Security Features/Serialization/DefiningPotentiallyUnsafeXmlSerializer.ql @@ -10,12 +10,17 @@ */ import csharp -import DataSetSerialization +deprecated import DataSetSerialization -from UnsafeXmlSerializerImplementation c, Member m -where +deprecated query predicate problems( + Member m, string message, UnsafeXmlSerializerImplementation c, string classMessage, Member member, + string memberMessage +) { c.fromSource() and - isClassUnsafeXmlSerializerImplementation(c, m) -select m, - "Defining an serializable class $@ that has member $@ of a type that is derived from DataSet or DataTable types and may lead to a security problem. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details.", - c, c.toString(), m, m.toString() + isClassUnsafeXmlSerializerImplementation(c, m) and + message = + "Defining an serializable class $@ that has member $@ of a type that is derived from DataSet or DataTable types and may lead to a security problem. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details." and + classMessage = c.toString() and + member = m and + memberMessage = m.toString() +} diff --git a/csharp/ql/src/experimental/Security Features/Serialization/UnsafeTypeUsedDataContractSerializer.ql b/csharp/ql/src/experimental/Security Features/Serialization/UnsafeTypeUsedDataContractSerializer.ql index 74eea14500d0..49245d3d8f56 100644 --- a/csharp/ql/src/experimental/Security Features/Serialization/UnsafeTypeUsedDataContractSerializer.ql +++ b/csharp/ql/src/experimental/Security Features/Serialization/UnsafeTypeUsedDataContractSerializer.ql @@ -10,7 +10,7 @@ */ import csharp -import DataSetSerialization +deprecated import DataSetSerialization predicate xmlSerializerConstructorArgument(Expr e) { exists(ObjectCreation oc, Constructor c | e = oc.getArgument(0) | @@ -21,7 +21,7 @@ predicate xmlSerializerConstructorArgument(Expr e) { ) } -predicate unsafeDataContractTypeCreation(Expr e) { +deprecated predicate unsafeDataContractTypeCreation(Expr e) { exists(MethodCall gt | gt.getTarget().getName() = "GetType" and e = gt and @@ -31,16 +31,20 @@ predicate unsafeDataContractTypeCreation(Expr e) { e.(TypeofExpr).getTypeAccess().getTarget() instanceof DataSetOrTableRelatedClass } -module FlowToDataSerializerConstructorConfig implements DataFlow::ConfigSig { +deprecated module FlowToDataSerializerConstructorConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node node) { unsafeDataContractTypeCreation(node.asExpr()) } predicate isSink(DataFlow::Node node) { xmlSerializerConstructorArgument(node.asExpr()) } } -module FlowToDataSerializerConstructor = DataFlow::Global; +deprecated module FlowToDataSerializerConstructor = + DataFlow::Global; -from DataFlow::Node source, DataFlow::Node sink -where FlowToDataSerializerConstructor::flow(source, sink) -select sink, - "Unsafe type is used in data contract serializer. Make sure $@ comes from the trusted source.", - source, source.toString() +deprecated query predicate problems( + DataFlow::Node sink, string message, DataFlow::Node source, string sourceMessage +) { + FlowToDataSerializerConstructor::flow(source, sink) and + message = + "Unsafe type is used in data contract serializer. Make sure $@ comes from the trusted source." and + sourceMessage = source.toString() +} diff --git a/csharp/ql/src/experimental/Security Features/Serialization/XmlDeserializationWithDataSet.ql b/csharp/ql/src/experimental/Security Features/Serialization/XmlDeserializationWithDataSet.ql index fbcba87bcf64..8e820135af10 100644 --- a/csharp/ql/src/experimental/Security Features/Serialization/XmlDeserializationWithDataSet.ql +++ b/csharp/ql/src/experimental/Security Features/Serialization/XmlDeserializationWithDataSet.ql @@ -10,8 +10,10 @@ */ import csharp -import DataSetSerialization +deprecated import DataSetSerialization -from UnsafeXmlReadMethodCall mc -select mc, - "Making an XML deserialization call with a type derived from DataSet or DataTable types and may lead to a security problem. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details." +deprecated query predicate problems(UnsafeXmlReadMethodCall mc, string message) { + message = + "Making an XML deserialization call with a type derived from DataSet or DataTable types and may lead to a security problem. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details." and + exists(mc) +} diff --git a/csharp/ql/src/experimental/Security Features/backdoor/DangerousNativeFunctionCall.ql b/csharp/ql/src/experimental/Security Features/backdoor/DangerousNativeFunctionCall.ql index b0f066ba70bf..347f3f507a37 100644 --- a/csharp/ql/src/experimental/Security Features/backdoor/DangerousNativeFunctionCall.ql +++ b/csharp/ql/src/experimental/Security Features/backdoor/DangerousNativeFunctionCall.ql @@ -48,8 +48,8 @@ predicate isExternMethod(Method externMethod) { SystemRuntimeInteropServicesComImportAttributeClass } -from MethodCall mc -where +deprecated query predicate problems(MethodCall mc, string message) { isExternMethod(mc.getTarget()) and - isDangerousMethod(mc.getTarget()) -select mc, "Call to an external method '" + mc.getTarget().getName() + "'." + isDangerousMethod(mc.getTarget()) and + message = "Call to an external method '" + mc.getTarget().getName() + "'." +} diff --git a/csharp/ql/src/experimental/Security Features/backdoor/PotentialTimeBomb.ql b/csharp/ql/src/experimental/Security Features/backdoor/PotentialTimeBomb.ql index 47b69d3e975d..6a4eeb002b33 100644 --- a/csharp/ql/src/experimental/Security Features/backdoor/PotentialTimeBomb.ql +++ b/csharp/ql/src/experimental/Security Features/backdoor/PotentialTimeBomb.ql @@ -174,13 +174,16 @@ predicate isPotentialTimeBomb( ) } -from - Flow::PathNode source, Flow::PathNode sink, Call getLastWriteTimeMethodCall, - Call timeArithmeticCall, Call timeComparisonCall, SelectionStmt selStatement -where +deprecated query predicate problems( + SelectionStmt selStatement, Flow::PathNode source, Flow::PathNode sink, string message, + Call timeComparisonCall, string timeComparisonCallString, Call timeArithmeticCall, string offset, + Call getLastWriteTimeMethodCall, string lastWriteTimeMethodCallMessage +) { isPotentialTimeBomb(source, sink, getLastWriteTimeMethodCall, timeArithmeticCall, - timeComparisonCall, selStatement) -select selStatement, source, sink, - "Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger.", - timeComparisonCall, timeComparisonCall.toString(), timeArithmeticCall, "offset", - getLastWriteTimeMethodCall, "last modification time of a file" + timeComparisonCall, selStatement) and + message = + "Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger." and + timeComparisonCallString = timeComparisonCall.toString() and + offset = "offset" and + lastWriteTimeMethodCallMessage = "last modification time of a file" +} diff --git a/csharp/ql/src/experimental/Security Features/backdoor/ProcessNameToHashTaintFlow.ql b/csharp/ql/src/experimental/Security Features/backdoor/ProcessNameToHashTaintFlow.ql index 2835ac1974a0..c8b23f476fd2 100644 --- a/csharp/ql/src/experimental/Security Features/backdoor/ProcessNameToHashTaintFlow.ql +++ b/csharp/ql/src/experimental/Security Features/backdoor/ProcessNameToHashTaintFlow.ql @@ -42,8 +42,15 @@ predicate isSuspiciousPropertyName(PropertyRead pr) { pr.getTarget().hasFullyQualifiedName("System.Diagnostics", "Process", "ProcessName") } -from DataFlowFromMethodToHash::PathNode src, DataFlowFromMethodToHash::PathNode sink -where DataFlowFromMethodToHash::flow(src.getNode(), sink.getNode()) -select src.getNode(), src, sink, - "The hash is calculated on $@, may be related to a backdoor. Please review the code for possible malicious intent.", - sink.getNode(), "this process name" +deprecated query predicate problems( + DataFlow::Node srcNode, DataFlowFromMethodToHash::PathNode src, + DataFlowFromMethodToHash::PathNode sink, string message, DataFlow::Node sinkNode, + string sinkMessage +) { + srcNode = src.getNode() and + sinkNode = sink.getNode() and + DataFlowFromMethodToHash::flow(srcNode, sinkNode) and + message = + "The hash is calculated on $@, may be related to a backdoor. Please review the code for possible malicious intent." and + sinkMessage = "this process name" +} diff --git a/csharp/ql/src/experimental/dataflow/flowsources/AuthCookie.qll b/csharp/ql/src/experimental/dataflow/flowsources/AuthCookie.qll index 401944adcc47..e91ae9de5385 100644 --- a/csharp/ql/src/experimental/dataflow/flowsources/AuthCookie.qll +++ b/csharp/ql/src/experimental/dataflow/flowsources/AuthCookie.qll @@ -1,6 +1,7 @@ /** * Provides classes and predicates for detecting insecure cookies. */ +deprecated module; import csharp import semmle.code.csharp.frameworks.microsoft.AspNetCore From 0932a0edb57cfa568a35fda3aace0dd2d4fe4d95 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 8 Nov 2024 09:41:24 +0100 Subject: [PATCH 2/3] C#: Updated expected test output. --- .../ql/test/experimental/CWE-918/RequestForgery.expected | 2 +- .../Security Features/CWE-759/HashWithoutSalt.expected | 8 ++++---- .../Security Features/backdoor/PotentialTimeBomb.expected | 2 +- .../backdoor/ProcessNameToHashTaintFlow.expected | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/csharp/ql/test/experimental/CWE-918/RequestForgery.expected b/csharp/ql/test/experimental/CWE-918/RequestForgery.expected index 37cc9ded82c8..d0e8e0281f84 100644 --- a/csharp/ql/test/experimental/CWE-918/RequestForgery.expected +++ b/csharp/ql/test/experimental/CWE-918/RequestForgery.expected @@ -4,5 +4,5 @@ nodes | RequestForgery.cs:12:52:12:54 | url : String | semmle.label | url : String | | RequestForgery.cs:14:66:14:68 | access to parameter url | semmle.label | access to parameter url | subpaths -#select +problems | RequestForgery.cs:14:66:14:68 | access to parameter url | RequestForgery.cs:12:52:12:54 | url : String | RequestForgery.cs:14:66:14:68 | access to parameter url | The URL of this request depends on a $@. | RequestForgery.cs:12:52:12:54 | url | user-provided value | diff --git a/csharp/ql/test/experimental/Security Features/CWE-759/HashWithoutSalt.expected b/csharp/ql/test/experimental/Security Features/CWE-759/HashWithoutSalt.expected index d381f4190fd3..07af4ac24f0a 100644 --- a/csharp/ql/test/experimental/Security Features/CWE-759/HashWithoutSalt.expected +++ b/csharp/ql/test/experimental/Security Features/CWE-759/HashWithoutSalt.expected @@ -1,7 +1,3 @@ -#select -| HashWithoutSalt.cs:20:49:20:56 | access to local variable passBuff | HashWithoutSalt.cs:18:70:18:77 | access to parameter password : String | HashWithoutSalt.cs:20:49:20:56 | access to local variable passBuff | $@ is hashed without a salt. | HashWithoutSalt.cs:18:70:18:77 | access to parameter password | The password | -| HashWithoutSalt.cs:39:51:39:59 | access to local variable passBytes | HashWithoutSalt.cs:38:64:38:71 | access to parameter password : String | HashWithoutSalt.cs:39:51:39:59 | access to local variable passBytes | $@ is hashed without a salt. | HashWithoutSalt.cs:38:64:38:71 | access to parameter password | The password | -| HashWithoutSalt.cs:71:48:71:56 | access to local variable passBytes | HashWithoutSalt.cs:70:64:70:71 | access to parameter password : String | HashWithoutSalt.cs:71:48:71:56 | access to local variable passBytes | $@ is hashed without a salt. | HashWithoutSalt.cs:70:64:70:71 | access to parameter password | The password | edges | HashWithoutSalt.cs:18:17:18:24 | access to local variable passBuff : IBuffer | HashWithoutSalt.cs:20:49:20:56 | access to local variable passBuff | provenance | | | HashWithoutSalt.cs:18:28:18:105 | call to method ConvertStringToBinary : IBuffer | HashWithoutSalt.cs:18:17:18:24 | access to local variable passBuff : IBuffer | provenance | | @@ -27,4 +23,8 @@ nodes | HashWithoutSalt.cs:70:28:70:72 | call to method GetBytes : Byte[] | semmle.label | call to method GetBytes : Byte[] | | HashWithoutSalt.cs:70:64:70:71 | access to parameter password : String | semmle.label | access to parameter password : String | | HashWithoutSalt.cs:71:48:71:56 | access to local variable passBytes | semmle.label | access to local variable passBytes | +problems +| HashWithoutSalt.cs:20:49:20:56 | access to local variable passBuff | HashWithoutSalt.cs:18:70:18:77 | access to parameter password : String | HashWithoutSalt.cs:20:49:20:56 | access to local variable passBuff | $@ is hashed without a salt. | HashWithoutSalt.cs:18:70:18:77 | access to parameter password | The password | +| HashWithoutSalt.cs:39:51:39:59 | access to local variable passBytes | HashWithoutSalt.cs:38:64:38:71 | access to parameter password : String | HashWithoutSalt.cs:39:51:39:59 | access to local variable passBytes | $@ is hashed without a salt. | HashWithoutSalt.cs:38:64:38:71 | access to parameter password | The password | +| HashWithoutSalt.cs:71:48:71:56 | access to local variable passBytes | HashWithoutSalt.cs:70:64:70:71 | access to parameter password : String | HashWithoutSalt.cs:71:48:71:56 | access to local variable passBytes | $@ is hashed without a salt. | HashWithoutSalt.cs:70:64:70:71 | access to parameter password | The password | subpaths diff --git a/csharp/ql/test/experimental/Security Features/backdoor/PotentialTimeBomb.expected b/csharp/ql/test/experimental/Security Features/backdoor/PotentialTimeBomb.expected index 3a2f86910fe6..95a491293ddb 100644 --- a/csharp/ql/test/experimental/Security Features/backdoor/PotentialTimeBomb.expected +++ b/csharp/ql/test/experimental/Security Features/backdoor/PotentialTimeBomb.expected @@ -17,7 +17,7 @@ edges | test.cs:71:36:71:70 | call to method AddHours | test.cs:71:13:71:71 | call to method CompareTo | provenance | | | test.cs:71:36:71:70 | call to method AddHours | test.cs:71:13:71:71 | call to method CompareTo : Int32 | provenance | | | test.cs:71:36:71:70 | call to method AddHours | test.cs:71:36:71:70 | call to method AddHours | provenance | | -#select +problems | test.cs:71:9:74:9 | if (...) ... | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:13:71:71 | call to method CompareTo | Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger. | test.cs:71:13:71:71 | call to method CompareTo | call to method CompareTo | test.cs:71:36:71:70 | call to method AddHours | offset | test.cs:69:34:69:76 | call to method GetLastWriteTime | last modification time of a file | | test.cs:71:9:74:9 | if (...) ... | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:13:71:71 | call to method CompareTo : Int32 | Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger. | test.cs:71:13:71:71 | call to method CompareTo | call to method CompareTo | test.cs:71:36:71:70 | call to method AddHours | offset | test.cs:69:34:69:76 | call to method GetLastWriteTime | last modification time of a file | | test.cs:71:9:74:9 | if (...) ... | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:13:71:76 | ... >= ... | Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger. | test.cs:71:13:71:71 | call to method CompareTo | call to method CompareTo | test.cs:71:36:71:70 | call to method AddHours | offset | test.cs:69:34:69:76 | call to method GetLastWriteTime | last modification time of a file | diff --git a/csharp/ql/test/experimental/Security Features/backdoor/ProcessNameToHashTaintFlow.expected b/csharp/ql/test/experimental/Security Features/backdoor/ProcessNameToHashTaintFlow.expected index e217064d1dfc..b8074b4fa636 100644 --- a/csharp/ql/test/experimental/Security Features/backdoor/ProcessNameToHashTaintFlow.expected +++ b/csharp/ql/test/experimental/Security Features/backdoor/ProcessNameToHashTaintFlow.expected @@ -1,4 +1,4 @@ edges nodes subpaths -#select +problems From 93562950bb53578d065bc69b581f15294ee4d314 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 8 Nov 2024 09:40:24 +0100 Subject: [PATCH 3/3] C#: Add change-note. --- csharp/ql/src/change-notes/2024-11-05-experimental-queries.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 csharp/ql/src/change-notes/2024-11-05-experimental-queries.md diff --git a/csharp/ql/src/change-notes/2024-11-05-experimental-queries.md b/csharp/ql/src/change-notes/2024-11-05-experimental-queries.md new file mode 100644 index 000000000000..f13df286191a --- /dev/null +++ b/csharp/ql/src/change-notes/2024-11-05-experimental-queries.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* All *experimental* queries have been deprecated. The queries are instead available as part of the *default* query suite in [CodeQL-Community-Packs](https://github.com/GitHubSecurityLab/CodeQL-Community-Packs). \ No newline at end of file