Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,26 +51,119 @@ predicate hasGlobalAntiForgeryFilter() {
)
}

predicate isUnvalidatedPostMethod(Class c, Method m) {
c.(Controller).getAPostActionMethod() = m and
not m.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute and
not c.getABaseType*().getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute
or
c.(AspNetCore::MicrosoftAspNetCoreMvcController).getAnActionMethod() = m and
m.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute and
not m.getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute and
not c.getABaseType*().getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute
private class RequireAntiforgeryTokenAttribute extends Attribute {
RequireAntiforgeryTokenAttribute() {
this.getType()
.hasFullyQualifiedName("Microsoft.AspNetCore.Antiforgery",
"RequireAntiforgeryTokenAttribute")
}

predicate requiresValidation() {
not exists(this.getArgument(0))
or
this.getArgument(0).isImplicit()
or
this.getArgument(0).getValue() = "true"
}
}

private predicate hasAspNetCoreAntiForgeryMiddleware() {
exists(MethodCall call |
call.getTarget()
.hasFullyQualifiedName("Microsoft.AspNetCore.Builder",
"AntiforgeryApplicationBuilderExtensions", "UseAntiforgery")
)
}

private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttributeOnMethod(
Method method
) {
exists(Method attributedMethod |
attributedMethod = method.getOverridee*() and
result = attributedMethod.getAnAttribute() and
not exists(Method closerMethod |
closerMethod = method.getOverridee*() and
closerMethod.getOverridee+() = attributedMethod and
closerMethod.getAnAttribute() instanceof RequireAntiforgeryTokenAttribute
)
)
}

private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttributeOnClass(
Class controller
) {
exists(Class attributedClass |
attributedClass = controller.getBaseClass*() and
result = attributedClass.getAnAttribute() and
not exists(Class closerClass |
closerClass = controller.getBaseClass*() and
closerClass.getBaseClass+() = attributedClass and
closerClass.getAnAttribute() instanceof RequireAntiforgeryTokenAttribute
)
)
}

class MvcControllerPostMethod extends Method {
private Controller controller;

MvcControllerPostMethod() { controller.getAPostActionMethod() = this }

predicate hasValidateAntiForgeryAttribute() {
this.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute or
controller.getABaseType*().getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute
}
}

class AspNetCoreControllerPostMethod extends Method {
private AspNetCore::MicrosoftAspNetCoreMvcController controller;

AspNetCoreControllerPostMethod() {
controller.getAnActionMethod() = this and
this.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute
}

predicate hasValidateAntiForgeryAttribute() {
this.getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute or
controller.getABaseType*().getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute
}

predicate hasRequireAntiForgeryAttribute() {
hasAspNetCoreAntiForgeryMiddleware() and
(
getEffectiveRequireAntiforgeryTokenAttributeOnMethod(this).requiresValidation()
or
not exists(getEffectiveRequireAntiforgeryTokenAttributeOnMethod(this)) and
getEffectiveRequireAntiforgeryTokenAttributeOnClass(controller).requiresValidation()
)
}
}

predicate isUnvalidatedAspNetCorePostMethod(AspNetCoreControllerPostMethod m) {
not m.hasValidateAntiForgeryAttribute() and
not m.hasRequireAntiForgeryAttribute()
}

predicate isUnvalidatedMvcPostMethod(MvcControllerPostMethod m) {
not m.hasValidateAntiForgeryAttribute()
}

predicate isUnvalidatedPostMethod(Method m) {
isUnvalidatedMvcPostMethod(m) or
isUnvalidatedAspNetCorePostMethod(m)
}

Element getAValidatedElement() {
any(ValidateAntiForgeryTokenAttribute a).getTarget() = result
or
any(AspNetCore::ValidateAntiForgeryAttribute a).getTarget() = result
or
hasAspNetCoreAntiForgeryMiddleware() and
any(RequireAntiforgeryTokenAttribute a | a.requiresValidation()).getTarget() = result
}

from Class c, Method postMethod
from Method postMethod
where
isUnvalidatedPostMethod(c, postMethod) and
isUnvalidatedPostMethod(postMethod) and
// Verify that validate anti forgery token attributes are used somewhere within this project, to
// avoid reporting false positives on projects that use an alternative approach to mitigate CSRF
// issues.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `cs/web/missing-token-validation` query now recognizes enabled ASP.NET Core `RequireAntiforgeryToken` attributes when antiforgery middleware is used.
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using Microsoft.AspNetCore.Antiforgery;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Mvc;

public class HomeController : Controller
{
[HttpPost]
[RequireAntiforgeryToken(false)]
public ActionResult DisabledValidation()
{
return View();
}

[HttpPost]
public ActionResult MissingValidation()
{
return View();
}
}

public class Startup
{
public void Configure(IApplicationBuilder app)
{
app.UseAntiforgery();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
query: Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
semmle-extractor-options: /nostdlib /noconfig
semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/_frameworks/Microsoft.AspNetCore.App/Microsoft.AspNetCore.App.csproj
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Antiforgery;
using Microsoft.AspNetCore.Builder;

public class HomeController : Controller
{
private const bool ValidationEnabled = true;
private const bool ValidationDisabled = false;

// BAD: Anti forgery token has been forgotten
[HttpPost]
public ActionResult Login() // $ Alert
Expand All @@ -17,6 +22,46 @@ public ActionResult UpdateDetails()
return View();
}

// GOOD: Anti forgery token is required by ASP.NET Core middleware
[HttpPost]
[RequireAntiforgeryToken]
public ActionResult UpdateProfile()
{
return View();
}

// GOOD: Explicitly requires anti forgery validation
[HttpPost]
[RequireAntiforgeryToken(true)]
public ActionResult UpdatePreferences()
{
return View();
}

// GOOD: Named and constant arguments are supported
[HttpPost]
[RequireAntiforgeryToken(required: ValidationEnabled)]
public ActionResult UpdateSettings()
{
return View();
}

// BAD: Explicitly disables anti forgery validation
[HttpPost]
[RequireAntiforgeryToken(false)]
public ActionResult DisabledValidation() // $ Alert
{
return View();
}

// BAD: A false constant also disables anti forgery validation
[HttpPost]
[RequireAntiforgeryToken(ValidationDisabled)]
public ActionResult ConstantDisabledValidation() // $ Alert
{
return View();
}

// No validation required, as this is a GET method.
public ActionResult ShowHelp()
{
Expand Down Expand Up @@ -46,6 +91,110 @@ public ActionResult InheritedValidation()
}
}

// GOOD: Base class requires anti forgery validation
[RequireAntiforgeryToken]
public abstract class AntiforgeryBaseController : Controller
{
}

public abstract class IntermediateAntiforgeryController : AntiforgeryBaseController
{
}

public class DerivedAntiforgeryController : IntermediateAntiforgeryController
{
[HttpPost]
public ActionResult InheritedRequiredValidation()
{
return View();
}
}

[RequireAntiforgeryToken]
public class ProtectedController : Controller
{
// GOOD: Controller requires anti forgery validation
[HttpPost]
public ActionResult ProtectedAction()
{
return View();
}

// BAD: Action-level metadata overrides the controller metadata
[HttpPost]
[RequireAntiforgeryToken(false)]
public ActionResult DisabledAction() // $ Alert
{
return View();
}
}

[RequireAntiforgeryToken(false)]
public class DisabledController : Controller
{
// BAD: Controller explicitly disables anti forgery validation
[HttpPost]
public ActionResult DisabledControllerAction() // $ Alert
{
return View();
}

// GOOD: Action-level metadata overrides the controller metadata
[HttpPost]
[RequireAntiforgeryToken(true)]
public ActionResult EnabledAction()
{
return View();
}
}

[RequireAntiforgeryToken]
public abstract class ProtectedBaseController : Controller
{
}

[RequireAntiforgeryToken(false)]
public class DisabledDerivedController : ProtectedBaseController
{
// BAD: Derived controller metadata overrides base controller metadata
[HttpPost]
public ActionResult DisabledInheritedAction() // $ Alert
{
return View();
}
}

[AutoValidateAntiforgeryToken]
public class FilterProtectedController : Controller
{
// GOOD: Disabled middleware metadata does not disable the MVC filter
[HttpPost]
[RequireAntiforgeryToken(false)]
public ActionResult FilterProtectedAction()
{
return View();
}
}

public abstract class MethodMetadataBaseController : Controller
{
[RequireAntiforgeryToken]
public virtual ActionResult InheritedMethodValidation()
{
return View();
}
}

public class MethodMetadataController : MethodMetadataBaseController
{
// GOOD: Method metadata is inherited by the override
[HttpPost]
public override ActionResult InheritedMethodValidation()
{
return View();
}
}

// BAD: Base class without antiforgery attribute
public abstract class UnprotectedBaseController : Controller
{
Expand All @@ -60,3 +209,29 @@ public ActionResult NoInheritedValidation() // $ Alert
return View();
}
}

namespace Custom
{
public class RequireAntiforgeryTokenAttribute : System.Attribute
{
}

public class CustomAttributeController : Controller
{
// BAD: An unrelated attribute with the same name does not provide validation
[HttpPost]
[RequireAntiforgeryToken]
public ActionResult LookalikeAttribute() // $ Alert
{
return View();
}
}
}

public class Startup
{
public void Configure(IApplicationBuilder app)
{
app.UseAntiforgery();
}
}
Original file line number Diff line number Diff line change
@@ -1,2 +1,8 @@
| MissingAntiForgeryTokenValidation.cs:7:25:7:29 | Login | Method 'Login' handles a POST request without performing CSRF token validation. |
| MissingAntiForgeryTokenValidation.cs:58:25:58:45 | NoInheritedValidation | Method 'NoInheritedValidation' handles a POST request without performing CSRF token validation. |
| MissingAntiForgeryTokenValidation.cs:12:25:12:29 | Login | Method 'Login' handles a POST request without performing CSRF token validation. |
| MissingAntiForgeryTokenValidation.cs:52:25:52:42 | DisabledValidation | Method 'DisabledValidation' handles a POST request without performing CSRF token validation. |
| MissingAntiForgeryTokenValidation.cs:60:25:60:50 | ConstantDisabledValidation | Method 'ConstantDisabledValidation' handles a POST request without performing CSRF token validation. |
| MissingAntiForgeryTokenValidation.cs:126:25:126:38 | DisabledAction | Method 'DisabledAction' handles a POST request without performing CSRF token validation. |
| MissingAntiForgeryTokenValidation.cs:137:25:137:48 | DisabledControllerAction | Method 'DisabledControllerAction' handles a POST request without performing CSRF token validation. |
| MissingAntiForgeryTokenValidation.cs:161:25:161:47 | DisabledInheritedAction | Method 'DisabledInheritedAction' handles a POST request without performing CSRF token validation. |
| MissingAntiForgeryTokenValidation.cs:207:25:207:45 | NoInheritedValidation | Method 'NoInheritedValidation' handles a POST request without performing CSRF token validation. |
| MissingAntiForgeryTokenValidation.cs:224:29:224:46 | LookalikeAttribute | Method 'LookalikeAttribute' handles a POST request without performing CSRF token validation. |
Loading
Loading