-
Notifications
You must be signed in to change notification settings - Fork 13
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #90 from GitHubSecurityLab/cors-csharp
Add CORS query to C# pack
- Loading branch information
Showing
13 changed files
with
419 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
<!DOCTYPE qhelp PUBLIC | ||
"-//Semmle//qhelp//EN" | ||
"qhelp.dtd"> | ||
<qhelp> | ||
|
||
<overview> | ||
<p> | ||
|
||
A server can send the | ||
<code>"Access-Control-Allow-Credentials"</code> CORS header to control | ||
when a browser may send user credentials in Cross-Origin HTTP | ||
requests. | ||
|
||
</p> | ||
<p> | ||
|
||
When the <code>Access-Control-Allow-Credentials</code> header | ||
is <code>"true"</code>, the <code>Access-Control-Allow-Origin</code> | ||
header must have a value different from <code>"*"</code> in order to | ||
make browsers accept the header. Therefore, to allow multiple origins | ||
for Cross-Origin requests with credentials, the server must | ||
dynamically compute the value of the | ||
<code>"Access-Control-Allow-Origin"</code> header. Computing this | ||
header value from information in the request to the server can | ||
therefore potentially allow an attacker to control the origins that | ||
the browser sends credentials to. | ||
|
||
</p> | ||
|
||
|
||
|
||
</overview> | ||
|
||
<recommendation> | ||
<p> | ||
|
||
When the <code>Access-Control-Allow-Credentials</code> header | ||
value is <code>"true"</code>, a dynamic computation of the | ||
<code>Access-Control-Allow-Origin</code> header must involve | ||
sanitization if it relies on user-controlled input. | ||
|
||
|
||
</p> | ||
<p> | ||
|
||
Since the <code>"null"</code> origin is easy to obtain for an | ||
attacker, it is never safe to use <code>"null"</code> as the value of | ||
the <code>Access-Control-Allow-Origin</code> header when the | ||
<code>Access-Control-Allow-Credentials</code> header value is | ||
<code>"true"</code>. | ||
|
||
</p> | ||
</recommendation> | ||
|
||
<example> | ||
<p> | ||
|
||
In the example below, the server allows the browser to send | ||
user credentials in a Cross-Origin request. The request header | ||
<code>origins</code> controls the allowed origins for such a | ||
Cross-Origin request. | ||
|
||
</p> | ||
|
||
<sample src="examples/CorsBad.cs"/> | ||
|
||
<p> | ||
|
||
This is not secure, since an attacker can choose the value of | ||
the <code>origin</code> request header to make the browser send | ||
credentials to their own server. The use of a allowlist containing | ||
allowed origins for the Cross-Origin request fixes the issue: | ||
|
||
</p> | ||
|
||
<sample src="examples/CorsGood.cs"/> | ||
</example> | ||
|
||
<references> | ||
<li>Mozilla Developer Network: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin">CORS, Access-Control-Allow-Origin</a>.</li> | ||
<li>Mozilla Developer Network: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials">CORS, Access-Control-Allow-Credentials</a>.</li> | ||
<li>PortSwigger: <a href="http://blog.portswigger.net/2016/10/exploiting-cors-misconfigurations-for.html">Exploiting CORS Misconfigurations for Bitcoins and Bounties</a></li> | ||
<li>W3C: <a href="https://w3c.github.io/webappsec-cors-for-developers/#resources">CORS for developers, Advice for Resource Owners</a></li> | ||
</references> | ||
</qhelp> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
/** | ||
* @name Credentialed CORS Misconfiguration | ||
* @description Allowing any origin while allowing credentials may result in security issues as third party website may be able to | ||
* access private resources. | ||
* @kind problem | ||
* @problem.severity error | ||
* @security-severity 7.5 | ||
* @precision high | ||
* @id cs/web/cors-misconfiguration | ||
* @tags security | ||
* external/cwe/cwe-942 | ||
*/ | ||
|
||
import csharp | ||
import CorsMisconfigurationLib | ||
|
||
/** | ||
* Holds if the application allows an origin using "*" origin. | ||
*/ | ||
private predicate allowAnyOrigin(MethodCall m) { | ||
m.getTarget() | ||
.hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", | ||
"AllowAnyOrigin") | ||
} | ||
|
||
/** | ||
* Holds if the application uses a vulnerable CORS policy. | ||
*/ | ||
private predicate hasDangerousOrigins(MethodCall m) { | ||
m.getTarget() | ||
.hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", | ||
"WithOrigins") and | ||
exists(StringLiteral idStr | | ||
idStr.getValue().toLowerCase().matches(["null", "*"]) and | ||
TaintTracking::localExprTaint(idStr, m.getAnArgument()) | ||
) | ||
} | ||
|
||
from MethodCall add_policy, MethodCall child | ||
where | ||
( | ||
usedPolicy(add_policy) and | ||
// Misconfigured origin affects used policy | ||
getCallableFromExpr(add_policy.getArgument(1)).calls*(child.getTarget()) | ||
or | ||
add_policy | ||
.getTarget() | ||
.hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions", | ||
"AddDefaultPolicy") and | ||
// Misconfigured origin affects added default policy | ||
getCallableFromExpr(add_policy.getArgument(0)).calls*(child.getTarget()) | ||
) and | ||
(hasDangerousOrigins(child) or allowAnyOrigin(child)) | ||
select add_policy, "The following CORS policy may allow requests from 3rd party websites" |
42 changes: 42 additions & 0 deletions
42
csharp/src/security/CWE-942/CorsMisconfigurationCredentials.ql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
/** | ||
* @name Credentialed CORS Misconfiguration | ||
* @description Allowing any origin while allowing credentials may result in security issues as third party website may be able to | ||
* access private resources. | ||
* @kind problem | ||
* @problem.severity error | ||
* @security-severity 7.5 | ||
* @precision high | ||
* @id cs/web/cors-misconfiguration-credentials | ||
* @tags security | ||
* external/cwe/cwe-942 | ||
*/ | ||
|
||
import csharp | ||
import CorsMisconfigurationLib | ||
|
||
/** A call to `CorsPolicyBuilder.AllowCredentials`. */ | ||
class AllowsCredentials extends MethodCall { | ||
AllowsCredentials() { | ||
this.getTarget() | ||
.hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", | ||
"AllowCredentials") | ||
} | ||
} | ||
|
||
from MethodCall add_policy, MethodCall setIsOriginAllowed, AllowsCredentials allowsCredentials | ||
where | ||
( | ||
getCallableFromExpr(add_policy.getArgument(1)).calls*(setIsOriginAllowed.getTarget()) and | ||
usedPolicy(add_policy) and | ||
getCallableFromExpr(add_policy.getArgument(1)).calls*(allowsCredentials.getTarget()) | ||
or | ||
add_policy | ||
.getTarget() | ||
.hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions", | ||
"AddDefaultPolicy") and | ||
getCallableFromExpr(add_policy.getArgument(0)).calls*(setIsOriginAllowed.getTarget()) and | ||
getCallableFromExpr(add_policy.getArgument(0)).calls*(allowsCredentials.getTarget()) | ||
) and | ||
setIsOriginAllowedReturnsTrue(setIsOriginAllowed) | ||
select add_policy, | ||
"The following CORS policy may allow credentialed requests from 3rd party websites" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import csharp | ||
import DataFlow | ||
import security.JsonWebTokenHandler.JsonWebTokenHandlerLib | ||
|
||
/** | ||
* Gets the actual callable corresponding to the expression `e`. | ||
*/ | ||
Callable getCallableFromExpr(Expr e) { | ||
exists(Expr dcArg | dcArg = e.(DelegateCreation).getArgument() | | ||
result = dcArg.(CallableAccess).getTarget() or | ||
result = dcArg.(AnonymousFunctionExpr) | ||
) | ||
or | ||
result = e | ||
} | ||
|
||
/** | ||
* Holds if SetIsOriginAllowed always returns true. This sets the Access-Control-Allow-Origin to the requester | ||
*/ | ||
predicate setIsOriginAllowedReturnsTrue(MethodCall mc) { | ||
mc.getTarget() | ||
.hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", | ||
"SetIsOriginAllowed") and | ||
mc.getArgument(0) instanceof CallableAlwaysReturnsTrue | ||
} | ||
|
||
/** | ||
* Holds if UseCors is called with the relevant cors policy | ||
*/ | ||
predicate usedPolicy(MethodCall add_policy) { | ||
exists(MethodCall uc | | ||
uc.getTarget() | ||
.hasFullyQualifiedName("Microsoft.AspNetCore.Builder.CorsMiddlewareExtensions", "UseCors") and | ||
( | ||
// Same hardcoded name | ||
uc.getArgument(1).getValue() = add_policy.getArgument(0).getValue() or | ||
// Same variable access | ||
uc.getArgument(1).(VariableAccess).getTarget() = | ||
add_policy.getArgument(0).(VariableAccess).getTarget() or | ||
DataFlow::localExprFlow(add_policy.getArgument(0), uc.getArgument(1)) | ||
) | ||
) and | ||
add_policy | ||
.getTarget() | ||
.hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions", "AddPolicy") | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
using Leaf.Middlewares; | ||
using Microsoft.AspNetCore.Builder; | ||
using Microsoft.AspNetCore.Hosting; | ||
using Microsoft.Extensions.Configuration; | ||
using Microsoft.Extensions.DependencyInjection; | ||
using Microsoft.Extensions.Hosting; | ||
|
||
namespace Leaf | ||
{ | ||
public class Startup | ||
{ | ||
public Startup(IConfiguration configuration) | ||
{ | ||
Configuration = configuration; | ||
} | ||
|
||
public IConfiguration Configuration { get; } | ||
|
||
// This method gets called by the runtime. Use this method to add services to the container. | ||
public void ConfigureServices(IServiceCollection services) | ||
{ | ||
services.AddControllers(); | ||
//services.AddTransient<MySqlConnection>(_ => new MySqlConnection(Configuration["ConnectionStrings:Default"])); | ||
services.AddControllersWithViews() | ||
.AddNewtonsoftJson(options => | ||
options.SerializerSettings.ReferenceLoopHandling = Newtonsoft.Json.ReferenceLoopHandling.Ignore | ||
); | ||
|
||
services.AddCors(options => { | ||
options.AddPolicy("AllowPolicy", builder => builder | ||
.WithOrigins("null") | ||
.AllowCredentials() | ||
.AllowAnyMethod() | ||
.AllowAnyHeader()); | ||
}); | ||
|
||
} | ||
|
||
// This method gets called by the runtime. Use this method to configure the HTTP request pipeline. | ||
public void Configure(IApplicationBuilder app, IWebHostEnvironment env) | ||
{ | ||
app.UseRouting(); | ||
|
||
app.UseCors("AllowPolicy"); | ||
|
||
app.UseRequestResponseLogging(); | ||
|
||
if (env.IsDevelopment()) | ||
{ | ||
app.UseDeveloperExceptionPage(); | ||
} | ||
|
||
app.UseHttpsRedirection(); | ||
|
||
app.UseAuthorization(); | ||
|
||
app.UseEndpoints(endpoints => | ||
{ | ||
endpoints.MapControllers(); | ||
}); | ||
|
||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
using Leaf.Middlewares; | ||
using Microsoft.AspNetCore.Builder; | ||
using Microsoft.AspNetCore.Hosting; | ||
using Microsoft.Extensions.Configuration; | ||
using Microsoft.Extensions.DependencyInjection; | ||
using Microsoft.Extensions.Hosting; | ||
|
||
namespace Leaf | ||
{ | ||
public class Startup | ||
{ | ||
public Startup(IConfiguration configuration) | ||
{ | ||
Configuration = configuration; | ||
} | ||
|
||
public IConfiguration Configuration { get; } | ||
|
||
// This method gets called by the runtime. Use this method to add services to the container. | ||
public void ConfigureServices(IServiceCollection services) | ||
{ | ||
services.AddControllers(); | ||
//services.AddTransient<MySqlConnection>(_ => new MySqlConnection(Configuration["ConnectionStrings:Default"])); | ||
services.AddControllersWithViews() | ||
.AddNewtonsoftJson(options => | ||
options.SerializerSettings.ReferenceLoopHandling = Newtonsoft.Json.ReferenceLoopHandling.Ignore | ||
); | ||
|
||
services.AddCors(options => { | ||
options.AddPolicy("AllowPolicy", builder => builder | ||
.WithOrigins("http://example.com") | ||
.AllowCredentials() | ||
.AllowAnyMethod() | ||
.AllowAnyHeader()); | ||
}); | ||
|
||
} | ||
|
||
// This method gets called by the runtime. Use this method to configure the HTTP request pipeline. | ||
public void Configure(IApplicationBuilder app, IWebHostEnvironment env) | ||
{ | ||
app.UseRouting(); | ||
|
||
app.UseCors("AllowPolicy"); | ||
|
||
app.UseRequestResponseLogging(); | ||
|
||
if (env.IsDevelopment()) | ||
{ | ||
app.UseDeveloperExceptionPage(); | ||
} | ||
|
||
app.UseHttpsRedirection(); | ||
|
||
app.UseAuthorization(); | ||
|
||
app.UseEndpoints(endpoints => | ||
{ | ||
endpoints.MapControllers(); | ||
}); | ||
|
||
} | ||
} | ||
} |
25 changes: 25 additions & 0 deletions
25
csharp/test/security/CWE-942/CorsMiconfigurationCredentials.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
using Microsoft.AspNetCore.Builder; | ||
using Microsoft.AspNetCore.Mvc; | ||
using System; | ||
using Microsoft.Extensions.DependencyInjection; | ||
|
||
public class Startup { | ||
public void ConfigureServices(string[] args) { | ||
var builder = WebApplication.CreateBuilder(args); | ||
var MyAllowSpecificOrigins = "_myAllowSpecificOrigins"; | ||
|
||
builder.Services.AddCors(options => { | ||
options.AddPolicy(MyAllowSpecificOrigins, | ||
policy => { | ||
policy.SetIsOriginAllowed(test => true).AllowCredentials().AllowAnyHeader().AllowAnyMethod(); | ||
}); | ||
}); | ||
|
||
var app = builder.Build(); | ||
|
||
app.MapGet("/", () => "Hello World!"); | ||
app.UseCors(MyAllowSpecificOrigins); | ||
|
||
app.Run(); | ||
} | ||
} |
Oops, something went wrong.