hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-27 01:35:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

C#: Re-factor on AppendCookieTracking to use the new API.

Browse Source
This commit is contained in:
Michael Nebel
2023-04-19 12:04:21 +02:00
parent 1b128a21e6
commit f976eeb909
3 changed files with 90 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
csharp/ql/src/experimental/Security Features/CWE-1004/CookieWithoutHttpOnly.ql
View File

@@ -36,7 +36,7 @@ where
iResponse.getAppendMethod() = mc.getTarget() and
isCookieWithSensitiveName(mc.getArgument(0)) and
// there is no callback `OnAppendCookie` that sets `HttpOnly` to true
not exists(OnAppendCookieHttpOnlyTrackingConfig config | config.hasFlowTo(_)) and
not OnAppendCookieHttpOnlyTracking::flowTo(_) and
// Passed as third argument to `IResponseCookies.Append`
exists(DataFlow::Node creation, DataFlow::Node append |
CookieOptionsTracking::flow(creation, append) and
@@ -67,7 +67,7 @@ where
// 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 exists(OnAppendCookieHttpOnlyTrackingConfig config | config.hasFlowTo(_)) and
not OnAppendCookieHttpOnlyTracking::flowTo(_) and
iResponse.getAppendMethod() = mc.getTarget() and
isCookieWithSensitiveName(mc.getArgument(0)) and
(

4
csharp/ql/src/experimental/Security Features/CWE-614/CookieWithoutSecure.ql
View File

@@ -30,7 +30,7 @@ where
getAValueForCookiePolicyProp("Secure").getValue() = "1"
) and
// there is no callback `OnAppendCookie` that sets `Secure` to true
not exists(OnAppendCookieSecureTrackingConfig config | config.hasFlowTo(_)) and
not OnAppendCookieSecureTracking::flowTo(_) and
(
// `Secure` property in `CookieOptions` passed to IResponseCookies.Append(...) wasn't set
exists(ObjectCreation oc |
@@ -80,7 +80,7 @@ where
or
oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and
// there is no callback `OnAppendCookie` that sets `Secure` to true
not exists(OnAppendCookieSecureTrackingConfig config | config.hasFlowTo(_)) and
not OnAppendCookieSecureTracking::flowTo(_) and
// the cookie option is passed to `Append`
exists(DataFlow::Node creation |
CookieOptionsTracking::flow(creation, _) and

88
csharp/ql/src/experimental/dataflow/flowsources/AuthCookie.qll
View File

@@ -135,18 +135,22 @@ Expr getAValueForProp(ObjectCreation create, Assignment a, string prop) {
predicate isPropertySet(ObjectCreation oc, string prop) { exists(getAValueForProp(oc, _, prop)) }
/**
* DEPRECATED: Use `OnAppendCookieSecureTracking` instead.
*
* Tracks if a callback used in `OnAppendCookie` sets `Secure` to `true`.
*/
class OnAppendCookieSecureTrackingConfig extends OnAppendCookieTrackingConfig {
deprecated class OnAppendCookieSecureTrackingConfig extends OnAppendCookieTrackingConfig {
OnAppendCookieSecureTrackingConfig() { this = "OnAppendCookieSecureTrackingConfig" }
override string propertyName() { result = "Secure" }
}
/**
* DEPRECATED: Use `OnAppendCookieHttpOnlyTracking` instead.
*
* Tracks if a callback used in `OnAppendCookie` sets `HttpOnly` to `true`.
*/
class OnAppendCookieHttpOnlyTrackingConfig extends OnAppendCookieTrackingConfig {
deprecated class OnAppendCookieHttpOnlyTrackingConfig extends OnAppendCookieTrackingConfig {
OnAppendCookieHttpOnlyTrackingConfig() { this = "OnAppendCookieHttpOnlyTrackingConfig" }
override string propertyName() { result = "HttpOnly" }
@@ -206,3 +210,83 @@ abstract private class OnAppendCookieTrackingConfig extends DataFlow::Configurat
)
}
}
private signature string propertyName();
/**
* Configuration for tracking if a callback used in `OnAppendCookie` sets a cookie property to `true`.
*/
private module OnAppendCookieTrackingConfig<propertyName/0 getPropertyName> implements
DataFlow::ConfigSig
{
/**
* Specifies the cookie property name to track.
*/
predicate isSource(DataFlow::Node source) {
exists(PropertyWrite pw, Assignment delegateAssign, Callable c |
pw.getProperty().getName() = "OnAppendCookie" and
pw.getProperty().getDeclaringType() instanceof MicrosoftAspNetCoreBuilderCookiePolicyOptions and
delegateAssign.getLValue() = pw and
(
exists(LambdaExpr lambda |
delegateAssign.getRValue() = lambda and
lambda = c
)
or
exists(DelegateCreation delegate |
delegateAssign.getRValue() = delegate and
delegate.getArgument().(CallableAccess).getTarget() = c
)
) and
c.getParameter(0) = source.asParameter()
)
}
predicate isSink(DataFlow::Node sink) {
exists(PropertyWrite pw, Assignment a |
pw.getProperty().getDeclaringType() instanceof MicrosoftAspNetCoreHttpCookieOptions and
pw.getProperty().getName() = getPropertyName() and
a.getLValue() = pw and
exists(Expr val |
DataFlow::localExprFlow(val, a.getRValue()) and
val.getValue() = "true"
) and
sink.asExpr() = pw.getQualifier()
)
}
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
node2.asExpr() =
any(PropertyRead pr |
pr.getQualifier() = node1.asExpr() and
pr.getProperty().getDeclaringType() instanceof
MicrosoftAspNetCoreCookiePolicyAppendCookieContext
)
}
}
private string getPropertyNameSecure() { result = "Secure" }
/**
* Configuration module for tracking if a callback used in `OnAppendCookie` sets `Secure` to `true`.
*/
private module OnAppendCookieSecureTrackingConfig =
OnAppendCookieTrackingConfig<getPropertyNameSecure/0>;
/**
* Tracks if a callback used in `OnAppendCookie` sets `Secure` to `true`.
*/
module OnAppendCookieSecureTracking = DataFlow::Global<OnAppendCookieSecureTrackingConfig>;
private string getPropertyNameHttpOnly() { result = "HttpOnly" }
/**
* Configuration module for tracking if a callback used in `OnAppendCookie` sets `HttpOnly` to `true`.
*/
private module OnAppendCookieHttpOnlyTrackingConfig =
OnAppendCookieTrackingConfig<getPropertyNameHttpOnly/0>;
/**
* Tracks if a callback used in `OnAppendCookie` sets `HttpOnly` to `true`.
*/
module OnAppendCookieHttpOnlyTracking = DataFlow::Global<OnAppendCookieHttpOnlyTrackingConfig>;