|
5 | 5 | * is used at all times. |
6 | 6 | * @kind problem |
7 | 7 | * @problem.severity error |
8 | | - * @precision medium |
9 | | - * @id cs/web/cookie-secure-not-set-aspnetcore |
| 8 | + * @precision high |
| 9 | + * @id cs/web/cookie-secure-not-set |
10 | 10 | * @tags security |
11 | 11 | * external/cwe/cwe-319 |
12 | 12 | * external/cwe/cwe-614 |
13 | 13 | */ |
14 | 14 |
|
15 | 15 | import csharp |
| 16 | +import semmle.code.asp.WebConfig |
| 17 | +import semmle.code.csharp.frameworks.system.Web |
16 | 18 | import semmle.code.csharp.frameworks.microsoft.AspNetCore |
17 | 19 | import semmle.code.csharp.dataflow.flowsources.AuthCookie |
18 | 20 |
|
19 | | -from Call c |
| 21 | +from Expr secureSink |
20 | 22 | where |
21 | | - // default is not configured or is not set to `Always` or `SameAsRequest` |
22 | | - not ( |
23 | | - getAValueForCookiePolicyProp("Secure").getValue() = "0" or |
24 | | - getAValueForCookiePolicyProp("Secure").getValue() = "1" |
25 | | - ) and |
26 | | - // there is no callback `OnAppendCookie` that sets `Secure` to true |
27 | | - not exists(OnAppendCookieSecureTrackingConfig config, DataFlow::Node source, DataFlow::Node sink | |
28 | | - config.hasFlow(source, sink) |
29 | | - ) and |
30 | | - ( |
31 | | - // `Secure` property in `CookieOptions` passed to IResponseCookies.Append(...) wasn't set |
32 | | - exists(ObjectCreation oc | |
33 | | - oc = c and |
34 | | - oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and |
35 | | - not isPropertySet(oc, "Secure") and |
36 | | - exists( |
37 | | - CookieOptionsTrackingConfiguration cookieTracking, DataFlow::Node creation, |
38 | | - DataFlow::Node append |
| 23 | + exists(Call c | |
| 24 | + secureSink = c and |
| 25 | + ( |
| 26 | + // default is not configured or is not set to `Always` or `SameAsRequest` |
| 27 | + not ( |
| 28 | + getAValueForCookiePolicyProp("Secure").getValue() = "0" or |
| 29 | + getAValueForCookiePolicyProp("Secure").getValue() = "1" |
| 30 | + ) and |
| 31 | + // there is no callback `OnAppendCookie` that sets `Secure` to true |
| 32 | + not exists( |
| 33 | + OnAppendCookieSecureTrackingConfig config, DataFlow::Node source, DataFlow::Node sink |
39 | 34 | | |
40 | | - cookieTracking.hasFlow(creation, append) and |
41 | | - creation.asExpr() = oc |
| 35 | + config.hasFlow(source, sink) |
| 36 | + ) and |
| 37 | + ( |
| 38 | + // `Secure` property in `CookieOptions` passed to IResponseCookies.Append(...) wasn't set |
| 39 | + exists(ObjectCreation oc | |
| 40 | + oc = c and |
| 41 | + oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and |
| 42 | + not isPropertySet(oc, "Secure") and |
| 43 | + exists( |
| 44 | + CookieOptionsTrackingConfiguration cookieTracking, DataFlow::Node creation, |
| 45 | + DataFlow::Node append |
| 46 | + | |
| 47 | + cookieTracking.hasFlow(creation, append) and |
| 48 | + creation.asExpr() = oc |
| 49 | + ) |
| 50 | + ) |
| 51 | + or |
| 52 | + // IResponseCookies.Append(String, String) was called, `Secure` is set to `false` by default |
| 53 | + exists(MethodCall mc, MicrosoftAspNetCoreHttpResponseCookies iResponse | |
| 54 | + mc = c and |
| 55 | + iResponse.getAppendMethod() = mc.getTarget() and |
| 56 | + mc.getNumberOfArguments() < 3 |
| 57 | + ) |
| 58 | + ) |
| 59 | + or |
| 60 | + exists(ObjectCreation oc | |
| 61 | + oc = c and |
| 62 | + oc.getType() instanceof SystemWebHttpCookie and |
| 63 | + // the property wasn't explicitly set, so a default value from config is used |
| 64 | + not isPropertySet(oc, "Secure") and |
| 65 | + // the default in config is not set to `true` |
| 66 | + not exists(XMLElement element | |
| 67 | + element instanceof FormsElement and |
| 68 | + element.(FormsElement).isRequireSSL() |
| 69 | + or |
| 70 | + element instanceof HttpCookiesElement and |
| 71 | + element.(HttpCookiesElement).isRequireSSL() |
| 72 | + ) |
42 | 73 | ) |
43 | 74 | ) |
44 | | - or |
45 | | - // IResponseCookies.Append(String, String) was called, `Secure` is set to `false` by default |
46 | | - exists(MethodCall mc, MicrosoftAspNetCoreHttpResponseCookies iResponse | |
47 | | - mc = c and |
48 | | - iResponse.getAppendMethod() = mc.getTarget() and |
49 | | - mc.getNumberOfArguments() < 3 |
| 75 | + ) |
| 76 | + or |
| 77 | + exists(Assignment a, Expr val | |
| 78 | + secureSink = a.getRValue() and |
| 79 | + ( |
| 80 | + exists(ObjectCreation oc | |
| 81 | + getAValueForProp(oc, a, "Secure") = val and |
| 82 | + val.getValue() = "false" and |
| 83 | + ( |
| 84 | + oc.getType() instanceof SystemWebHttpCookie |
| 85 | + or |
| 86 | + oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and |
| 87 | + // there is no callback `OnAppendCookie` that sets `Secure` to true |
| 88 | + not exists( |
| 89 | + OnAppendCookieSecureTrackingConfig config, DataFlow::Node source, DataFlow::Node sink |
| 90 | + | |
| 91 | + config.hasFlow(source, sink) |
| 92 | + ) and |
| 93 | + // the cookie option is passed to `Append` |
| 94 | + exists( |
| 95 | + CookieOptionsTrackingConfiguration cookieTracking, DataFlow::Node creation, |
| 96 | + DataFlow::Node append |
| 97 | + | |
| 98 | + cookieTracking.hasFlow(creation, append) and |
| 99 | + creation.asExpr() = oc |
| 100 | + ) |
| 101 | + ) |
| 102 | + ) |
| 103 | + or |
| 104 | + exists(PropertyWrite pw | |
| 105 | + ( |
| 106 | + pw.getProperty().getDeclaringType() instanceof MicrosoftAspNetCoreHttpCookieBuilder or |
| 107 | + pw.getProperty().getDeclaringType() instanceof |
| 108 | + MicrosoftAspNetCoreAuthenticationCookiesCookieAuthenticationOptions |
| 109 | + ) and |
| 110 | + pw.getProperty().getName() = "SecurePolicy" and |
| 111 | + a.getLValue() = pw and |
| 112 | + DataFlow::localExprFlow(val, a.getRValue()) and |
| 113 | + val.getValue() = "2" // None |
| 114 | + ) |
50 | 115 | ) |
51 | 116 | ) |
52 | | -select c, "Cookie attribute 'Secure' is not set to true." |
| 117 | +select secureSink, "Cookie attribute 'Secure' is not set to true." |
0 commit comments