CVE-2022-3167: Improper Restriction of Rendered UI Layers or Frames (CWE-1021)
value: Improper Restriction of Rendered UI Layers or Frames in GitHub repository ikus060/rdiffweb prior to 2.4.1.
CWE description: The web application does not restrict or incorrectly restricts frame objects or UI layers that belong to another application or domain, which can lead to user confusion about which interface the user is interacting with.
CVSS v3 base score: 8.8
Published: 2022-09-08T19:15Z
Repository: https://github.com/ikus060/rdiffweb
Language: Python
Files changed: README.md, rdiffweb/controller/tests/test_csrf.py, rdiffweb/tools/security.py
Commit date: 2022-09-08 08:32:32 -0400
Commit message: Add Clickjacking Defense

Diff: diff --git a/README.md b/README.md
index 2838f32..c042df5 100644
--- a/README.md
+++ b/README.md
@@ -105,10 +105,14 @@ Bug reports should be reported on the Rdiffweb Gitlab at https://gitlab.com/ikus
 
 Professional support for Rdiffweb is available by contacting [IKUS Soft](https://www.ikus-soft.com/en/support/#form).
 
 # Changelog
 
+## 2.4.1 (2022-09-08)
+
+* Add Clickjacking Defense
+
 ## 2.4.0 (2022-06-21)
 
 This new release brings a lot of improvement since the last version, multiple bug fixes
 to make the application stable. A couple of new features to improve the overall
 usability and a new security feature to block a brute force attack.
diff --git a/rdiffweb/controller/tests/test_csrf.py b/rdiffweb/controller/tests/test_csrf.py
index fd63162..af58d0c 100644
--- a/rdiffweb/controller/tests/test_csrf.py
+++ b/rdiffweb/controller/tests/test_csrf.py
@@ -69,5 +69,13 @@ class CsrfTest(rdiffweb.test.WebCase):
         # Given a POST request made to rdiffweb
         # When the request is made without an origin
         self.getPage('/', method='POST')
         # Then the request is accepted with 200 OK
         self.assertStatus(200)
+
+    def test_clickjacking_defense(self):
+        # Given a POST request made to rdiffweb
+        # When the request is made without an origin
+        self.getPage('/')
+        # Then the request is accepted with 200 OK
+        self.assertStatus(200)
+        self.assertHeaderItemValue('X-Frame-Options', 'DENY')
diff --git a/rdiffweb/tools/security.py b/rdiffweb/tools/security.py
index 0d31560..6fe6265 100644
--- a/rdiffweb/tools/security.py
+++ b/rdiffweb/tools/security.py
@@ -34,30 +34,36 @@ if not http.cookies.Morsel().isReservedKey("samesite"):
 
 class CsrfAuth(HandlerTool):
     """
     This tool provide CSRF mitigation.
 
-    First, by defining `SameSite=Lax` on the cookie
-    Second by validating the `Origin` and `Referer`.
+    * Define X-Frame-Options = DENY
+    * Define Cookies SameSite=Lax
+    * Validate `Origin` and `Referer` on POST, PUT, PATCH, DELETE
 
-    Ref.: https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html
+    Ref.:
+    https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html
+    https://cheatsheetseries.owasp.org/cheatsheets/Clickjacking_Defense_Cheat_Sheet.html
     """
 
     def __init__(self):
         HandlerTool.__init__(self, self.run, name='csrf')
         # Make sure to run before authform (priority 71)
         self._priority = 71
 
     def _setup(self):
-        cherrypy.request.hooks.attach('before_finalize', self._set_same_site)
+        cherrypy.request.hooks.attach('before_finalize', self._set_headers)
         return super()._setup()
 
-    def _set_same_site(self):
+    def _set_headers(self):
+        response = cherrypy.serving.response
+        # Define X-Frame-Options to avoid Clickjacking
+        response.headers['X-Frame-Options'] = 'DENY'
         # Awaiting bug fix in cherrypy
         # https://github.com/cherrypy/cherrypy/issues/1767
         # Force SameSite to Lax
-        cookie = cherrypy.serving.response.cookie.get('session_id', None)
+        cookie = response.cookie.get('session_id', None)
         if cookie:
             cookie['samesite'] = 'Lax'
 
     def run(self):
         if cherrypy.request.method in ['POST', 'PUT', 'PATCH', 'DELETE']:

Vulnerable code:     First, by defining `SameSite=Lax` on the cookie
    Second by validating the `Origin` and `Referer`.
    Ref.: https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html
        cherrypy.request.hooks.attach('before_finalize', self._set_same_site)
    def _set_same_site(self):
        cookie = cherrypy.serving.response.cookie.get('session_id', None)
Fixed code: ## 2.4.1 (2022-09-08)

* Add Clickjacking Defense


    def test_clickjacking_defense(self):
        # Given a POST request made to rdiffweb
        # When the request is made without an origin
        self.getPage('/')
        # Then the request is accepted with 200 OK
        self.assertStatus(200)
        self.assertHeaderItemValue('X-Frame-Options', 'DENY')
    * Define X-Frame-Options = DENY
    * Define Cookies SameSite=Lax
    * Validate `Origin` and `Referer` on POST, PUT, PATCH, DELETE
    Ref.:
    https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html
    https://cheatsheetseries.owasp.org/cheatsheets/Clickjacking_Defense_Cheat_Sheet.html
        cherrypy.request.hooks.attach('before_finalize', self._set_headers)
    def _set_headers(self):
        response = cherrypy.serving.response
        # Define X-Frame-Options to avoid Clickjacking
        response.headers['X-Frame-Options'] = 'DENY'
        cookie = response.cookie.get('session_id', None)