python: broaden local protection concept

Author: yoff

python/ql/lib/semmle/python/Concepts.qll

@@ -123,7 +123,7 @@ class CsrfProtectionSetting extends DataFlow::Node instanceof CsrfProtectionSett
 /** Provides a class for modeling new CSRF protection setting APIs. */
 module CsrfProtectionSetting {
   /**
-   * A data-flow node that may set or unset Cross-site request forgery protection
+   * A data-flow node that enables or disables Cross-site request forgery protection
    * in a global manner.
    *
    * Extend this class to model new APIs. If you want to refine existing API models,
@@ -139,35 +139,39 @@ module CsrfProtectionSetting {
 }
 
 /**
- * A data-flow node that provides Cross-site request forgery protection
+ * A data-flow node that enables or disables Cross-site request forgery protection
  * for a specific part of an application.
  *
  * Extend this class to refine existing API models. If you want to model new APIs,
- * extend `CsrfLocalProtection::Range` instead.
+ * extend `CsrfLocalProtectionSetting::Range` instead.
  */
-class CsrfLocalProtection extends DataFlow::Node instanceof CsrfLocalProtection::Range {
+class CsrfLocalProtectionSetting extends DataFlow::Node instanceof CsrfLocalProtectionSetting::Range {
   /**
-   * Gets a `Function` representing the protected interaction
-   * (probably a request handler).
+   * Gets a request handler whose CSRF protection is changed.
    */
-  Function getProtected() { result = super.getProtected() }
+  Function getRequestHandler() { result = super.getRequestHandler() }
+
+  /** Holds if CSRF protection is enabled by this setting */
+  predicate csrfEnabled() { super.csrfEnabled() }
 }
 
 /** Provides a class for modeling new CSRF protection setting APIs. */
-module CsrfLocalProtection {
+module CsrfLocalProtectionSetting {
   /**
-   * A data-flow node that provides Cross-site request forgery protection
+   * A data-flow node that enables or disables Cross-site request forgery protection
    * for a specific part of an application.
    *
    * Extend this class to model new APIs. If you want to refine existing API models,
-   * extend `CsrfLocalProtection` instead.
+   * extend `CsrfLocalProtectionSetting` instead.
    */
   abstract class Range extends DataFlow::Node {
     /**
-     * Gets a `Function` representing the protected interaction
-     * (probably a request handler).
+     * Gets a request handler whose CSRF protection is changed.
      */
-    abstract Function getProtected();
+    abstract Function getRequestHandler();
+
+    /** Holds if CSRF protection is enabled by this setting */
+    abstract predicate csrfEnabled();
   }
 }
 

python/ql/lib/semmle/python/frameworks/Django.qll

@@ -2356,20 +2356,24 @@ module PrivateDjango {
     }
   }
 
-  private class DjangoCsrfDecorator extends CsrfLocalProtection::Range {
+  private class DjangoCsrfDecorator extends CsrfLocalProtectionSetting::Range {
+    string decoratorName;
     Function function;
 
     DjangoCsrfDecorator() {
+      decoratorName in ["csrf_protect", "csrf_exempt", "requires_csrf_token", "ensure_csrf_cookie"] and
       this =
         API::moduleImport("django")
             .getMember("views")
             .getMember("decorators")
             .getMember("csrf")
-            .getMember("csrf_protect")
+            .getMember(decoratorName)
             .getAUse() and
       this.asExpr() = function.getADecorator()
     }
 
-    override Function getProtected() { result = function }
+    override Function getRequestHandler() { result = function }
+
+    override predicate csrfEnabled() { decoratorName in ["csrf_protect", "requires_csrf_token"] }
   }
 }

python/ql/src/Security/CWE-352/CSRFProtectionDisabled.ql

@@ -17,7 +17,7 @@ import semmle.python.Concepts
 from CsrfProtectionSetting s
 where
   s.getVerificationSetting() = false and
-  not exists(CsrfLocalProtection p) and
+  not exists(CsrfLocalProtectionSetting p | p.csrfEnabled()) and
   // rule out test code as this is a common place to turn off CSRF protection
   not s.getLocation().getFile().getAbsolutePath().matches("%test%")
 select s, "Potential CSRF vulnerability due to forgery protection being disabled or weakened."

python/ql/test/experimental/meta/ConceptsTest.qll

@@ -520,18 +520,20 @@ class CsrfProtectionSettingTest extends InlineExpectationsTest {
   }
 }
 
-class CsrfLocalProtectionTest extends InlineExpectationsTest {
-  CsrfLocalProtectionTest() { this = "CsrfLocalProtectionTest" }
+class CsrfLocalProtectionSettingTest extends InlineExpectationsTest {
+  CsrfLocalProtectionSettingTest() { this = "CsrfLocalProtectionSettingTest" }
 
-  override string getARelevantTag() { result = "CsrfLocalProtection" }
+  override string getARelevantTag() { result = "CsrfLocalProtection" + ["Enabled", "Disabled"] }
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     exists(location.getFile().getRelativePath()) and
-    exists(CsrfLocalProtection p |
+    exists(CsrfLocalProtectionSetting p |
       location = p.getLocation() and
       element = p.toString() and
-      value = p.getProtected().getName().toString() and
-      tag = "CsrfLocalProtection"
+      value = p.getRequestHandler().getName().toString() and
+      if p.csrfEnabled()
+      then tag = "CsrfLocalProtectionEnabled"
+      else tag = "CsrfLocalProtectionDisabled"
     )
   }
 }

python/ql/test/library-tests/frameworks/django-v2-v3/response_test.py

@@ -118,7 +118,7 @@ class CustomJsonResponse(JsonResponse):
     def __init__(self, banner, content, *args, **kwargs):
         super().__init__(content, *args, content_type="text/html", **kwargs)
 
-@csrf_protect  # $CsrfLocalProtection=safe__custom_json_response
+@csrf_protect  # $CsrfLocalProtectionEnabled=safe__custom_json_response
 def safe__custom_json_response(request):
     return CustomJsonResponse("ACME Responses", {"foo": request.GET.get("foo")})  # $HttpResponse mimetype=application/json MISSING: responseBody=Dict SPURIOUS: responseBody="ACME Responses"