CVE-2022-4646: Cross-Site Request Forgery (CSRF) (CWE-352)
value: Cross-Site Request Forgery (CSRF) in GitHub repository ikus060/rdiffweb prior to 2.5.4.
CWE description: The web application does not, or can not, sufficiently verify whether a well-formed, valid, consistent request was intentionally provided by the user who submitted the request.
CVSS v3 base score: 6.5
Published: 2022-12-22T02:15Z
Repository: https://github.com/ikus060/rdiffweb
Language: Python
Files changed: README.md, rdiffweb/controller/tests/test_controller.py, rdiffweb/controller/tests/test_page_admin_users.py, rdiffweb/controller/tests/test_page_login.py, rdiffweb/controller/tests/test_page_mfa.py, rdiffweb/controller/tests/test_secure_headers.py, rdiffweb/templates/layout.html, rdiffweb/templates/mfa.html, rdiffweb/test.py, rdiffweb/tools/auth_form.py
Commit date: 2022-12-19 08:18:41 -0500
Commit message: Add CSRF verification on `/logout`

Diff: diff --git a/README.md b/README.md
index e1a8a35f2..8fdc45e3e 100644
--- a/README.md
+++ b/README.md
@@ -109,11 +109,12 @@ Professional support for Rdiffweb is available by contacting [IKUS Soft](https:/
 # Changelog
 
 ## Next Release - 2.5.4
 
 * Discard `X-Forwarded-Host` headers credit to [Anishka Shukla](https://github.com/anishkashukla)
-* Create proper symbolic link of chartkick.js on Ubuntu Jammy to fix loading of Charts in web interface
+* Create proper symbolic link of `chartkick.js` on Ubuntu Jammy to fix loading of Charts in web interface
+* Add CSRF verification on `/logout` credits to [reza.duty](https://rezaduty.me)
 
 ## 2.5.3 (2022-12-05)
 
 * Add support for WTForms v3 to support Debian Bookworm
 * Fix strange behavior in access token management #247
diff --git a/rdiffweb/controller/tests/test_controller.py b/rdiffweb/controller/tests/test_controller.py
index 51e84b858..8badc0377 100644
--- a/rdiffweb/controller/tests/test_controller.py
+++ b/rdiffweb/controller/tests/test_controller.py
@@ -75,11 +75,11 @@ class ControllerTest(rdiffweb.test.WebCase):
     )
     def test_static_files(self, path):
         """
         Check if the theme is properly configure.
         """
-        self.getPage('/logout')
+        self.getPage('/logout', method="POST")
         self.getPage(path)
         self.assertStatus(200)
         # Test with invalid method.
         self.getPage(path, method="POST")
         self.assertStatus(400)
diff --git a/rdiffweb/controller/tests/test_page_admin_users.py b/rdiffweb/controller/tests/test_page_admin_users.py
index 55e8a1c32..f3bbdbfc0 100644
--- a/rdiffweb/controller/tests/test_page_admin_users.py
+++ b/rdiffweb/controller/tests/test_page_admin_users.py
@@ -323,11 +323,11 @@ class AbstractAdminTest(rdiffweb.test.WebCase):
         """
         Verify failure to delete our self.
         """
         # Create another admin user
         self._add_user('admin2', '', 'pr3j5Dwi', '', UserObject.ADMIN_ROLE)
-        self.getPage("/logout")
+        self.getPage("/logout", method="POST")
         self.assertStatus(303)
         self.assertHeaderItemValue('Location', self.baseurl + '/')
         self._login('admin2', 'pr3j5Dwi')
 
         # Try deleting admin user
diff --git a/rdiffweb/controller/tests/test_page_login.py b/rdiffweb/controller/tests/test_page_login.py
index 5aa89fc09..44ad9656a 100644
--- a/rdiffweb/controller/tests/test_page_login.py
+++ b/rdiffweb/controller/tests/test_page_login.py
@@ -181,11 +181,11 @@ class LoginPageTest(rdiffweb.test.WebCase):
         self.assertStatus(200)
         self.assertInBody(self.USERNAME)
 
     def test_login_persistent(self):
         # Given a user authenticated with persistent
-        self.getPage('/logout/')
+        self.getPage('/logout', method="POST")
         self.assertStatus(303)
         self.getPage(
             '/login/', method='POST', body={'login': self.USERNAME, 'password': self.PASSWORD, 'persistent': '1'}
         )
         self.assertStatus(303)
@@ -310,11 +310,11 @@ class LoginPageRateLimitTest3(rdiffweb.test.WebCase):
 
 class LogoutPageTest(rdiffweb.test.WebCase):
     def test_getpage_without_login(self):
         # Given an unauthenticated user
         # When Accessing logout page directly
-        self.getPage('/logout')
+        self.getPage('/logout', method="POST")
         # Then user is redirect to root '/'
         self.assertStatus('303 See Other')
         self.assertHeaderItemValue('Location', self.baseurl + '/')
 
     def test_getpage_with_login(self):
@@ -330,11 +330,11 @@ class LogoutPageTest(rdiffweb.test.WebCase):
         prev_session_id = self.session_id
         # Get content of a page.
         self.getPage("/prefs/general")
         self.assertStatus('200 OK')
         # When logout
-        self.getPage('/logout')
+        self.getPage('/logout', method="POST")
         # Then a new session id is generated
         self.assertNotEqual(prev_session_id, self.session_id)
         # Then user is redirected to root page
         self.assertStatus('303 See Other')
         self.assertHeaderItemValue('Location', self.baseurl + '/')
diff --git a/rdiffweb/controller/tests/test_page_mfa.py b/rdiffweb/controller/tests/test_page_mfa.py
index a84c8ab12..507d3d3e0 100644
--- a/rdiffweb/controller/tests/test_page_mfa.py
+++ b/rdiffweb/controller/tests/test_page_mfa.py
@@ -50,11 +50,11 @@ class MfaPageTest(rdiffweb.test.WebCase):
         userobj.email = 'admin@example.com'
         userobj.commit()
 
     def test_get_without_login(self):
         # Given an unauthenticated user
-        self.getPage("/logout")
+        self.getPage("/logout", method="POST")
         self.assertStatus(303)
         # When requesting /mfa/
         self.getPage("/mfa/")
         # Then user is redirected to /login/
         self.assertStatus(303)
diff --git a/rdiffweb/controller/tests/test_secure_headers.py b/rdiffweb/controller/tests/test_secure_headers.py
index 37538337f..9e81f79a3 100644
--- a/rdiffweb/controller/tests/test_secure_headers.py
+++ b/rdiffweb/controller/tests/test_secure_headers.py
@@ -56,11 +56,11 @@ class SecureHeadersTest(rdiffweb.test.WebCase):
     @parameterized.expand(
         [
             ('/invalid', 404),
             ('/browse/invalid', 404),
             ('/login', 301),
-            ('/logout', 303),
+            ('/logout', 405),
         ]
     )
     def test_cookie_with_https_http_error(self, url, expected_error_code):
         # Given an https request made to rdiffweb
         self.getPage(url, headers=[('X-Forwarded-Proto', 'https')])
diff --git a/rdiffweb/templates/layout.html b/rdiffweb/templates/layout.html
index 89b2445ad..aa5783832 100644
--- a/rdiffweb/templates/layout.html
+++ b/rdiffweb/templates/layout.html
@@ -83,17 +83,20 @@
                    id="navbarDropdown"
                    role="button">
                   <i aria-hidden="true" class="fa fa-user"></i>
                   {{ username }}
                 </a>
-                <div aria-labelledby="navbarDropdown" class="dropdown-menu">
-                  <h6 class="dropdown-header">{% trans %}Signed in as{% endtrans %} {{ username }}</h6>
-                  <a class="dropdown-item" href="{{ url_for('prefs', 'general') }}">{% trans %}Edit profile{% endtrans %}</a>
-                  <a class="dropdown-item" href="{{ url_for('prefs', 'notification') }}">{% trans %}Notifications{% endtrans %}</a>
-                  <div class="dropdown-divider"></div>
-                  <a class="dropdown-item" href="{{ url_for('logout') }}">Logout</a>
-                </div>
+                <form action="{{ url_for('logout') }}" method="post">
+                  <div aria-labelledby="navbarDropdown" class="dropdown-menu">
+                    <h6 class="dropdown-header">{% trans %}Signed in as{% endtrans %} {{ username }}</h6>
+                    <a class="dropdown-item" href="{{ url_for('prefs', 'general') }}">{% trans %}Edit profile{% endtrans %}</a>
+                    <a class="dropdown-item" href="{{ url_for('prefs', 'notification') }}">{% trans %}Notifications{% endtrans %}</a>
+                    <div class="dropdown-divider"></div>
+                    {# Logout button #}
+                    <button type="submit" class="dropdown-item">{% trans %}Logout{% endtrans %}</button>
+                  </div>
+                </form>
               </li>
             </ul>
           </div>
         </div>
       </nav>
diff --git a/rdiffweb/templates/mfa.html b/rdiffweb/templates/mfa.html
index 0a6ff7307..fbbae815a 100644
--- a/rdiffweb/templates/mfa.html
+++ b/rdiffweb/templates/mfa.html
@@ -9,13 +9,15 @@
           <p>
             {% trans %}Two-Factor Authentication is enabled for your account. To verify your account, you must enter the code that was sent to your email address.{% endtrans %}
           </p>
           {{ form }}
         </form>
-        <a class="btn-link btn-sm btn-block text-center"
-           href="{{ url_for('logout')}}">
-          {% trans %}Login with a different account{% endtrans %}
-        </a>
+        {# Logout button #}
+        <form action="{{ url_for('logout') }}" method="post">
+          <button type="submit" class="btn btn-link btn-sm btn-block text-center">
+            {% trans %}Login with a different account{% endtrans %}
+          </button>
+        </form>
       </div>
     </div>
   </div>
 {% endblock %}
diff --git a/rdiffweb/test.py b/rdiffweb/test.py
index 05e643815..c016c1962 100644
--- a/rdiffweb/test.py
+++ b/rdiffweb/test.py
@@ -174,11 +174,11 @@ class WebCase(helper.CPWebCase):
         self.getPage(*args, **kwargs)
         self.assertStatus(200)
         return json.loads(self.body.decode('utf8'))
 
     def _login(self, username=USERNAME, password=PASSWORD):
-        self.getPage("/logout")
+        self.getPage("/logout", method="POST")
         self.getPage("/login/", method='POST', body={'login': username, 'password': password})
         self.assertStatus('303 See Other')
 
     def wait_for_tasks(self):
         time.sleep(1)
diff --git a/rdiffweb/tools/auth_form.py b/rdiffweb/tools/auth_form.py
index 7a2442c9c..62047f44d 100644
--- a/rdiffweb/tools/auth_form.py
+++ b/rdiffweb/tools/auth_form.py
@@ -102,10 +102,12 @@ class CheckAuthForm(cherrypy.Tool):
                 raise cherrypy.HTTPRedirect('/')
             return
 
         # Clear session when browsing /logout
         if request.path_info == logout_url or request.path_info.startswith(logout_url):
+            if request.method != 'POST':
+                raise cherrypy.HTTPError(405)
             self.logout()
             raise cherrypy.HTTPRedirect('/')
 
         # Check if login
         if not self._is_login():

Vulnerable code: * Create proper symbolic link of chartkick.js on Ubuntu Jammy to fix loading of Charts in web interface
        self.getPage('/logout')
        self.getPage("/logout")
        self.getPage('/logout/')
        self.getPage('/logout')
        self.getPage('/logout')
        self.getPage("/logout")
            ('/logout', 303),
                <div aria-labelledby="navbarDropdown" class="dropdown-menu">
                  <h6 class="dropdown-header">{% trans %}Signed in as{% endtrans %} {{ username }}</h6>
                  <a class="dropdown-item" href="{{ url_for('prefs', 'general') }}">{% trans %}Edit profile{% endtrans %}</a>
                  <a class="dropdown-item" href="{{ url_for('prefs', 'notification') }}">{% trans %}Notifications{% endtrans %}</a>
                  <div class="dropdown-divider"></div>
                  <a class="dropdown-item" href="{{ url_for('logout') }}">Logout</a>
                </div>
        <a class="btn-link btn-sm btn-block text-center"
           href="{{ url_for('logout')}}">
          {% trans %}Login with a different account{% endtrans %}
        </a>
        self.getPage("/logout")
Fixed code: * Create proper symbolic link of `chartkick.js` on Ubuntu Jammy to fix loading of Charts in web interface
* Add CSRF verification on `/logout` credits to [reza.duty](https://rezaduty.me)
        self.getPage('/logout', method="POST")
        self.getPage("/logout", method="POST")
        self.getPage('/logout', method="POST")
        self.getPage('/logout', method="POST")
        self.getPage('/logout', method="POST")
        self.getPage("/logout", method="POST")
            ('/logout', 405),
                <form action="{{ url_for('logout') }}" method="post">
                  <div aria-labelledby="navbarDropdown" class="dropdown-menu">
                    <h6 class="dropdown-header">{% trans %}Signed in as{% endtrans %} {{ username }}</h6>
                    <a class="dropdown-item" href="{{ url_for('prefs', 'general') }}">{% trans %}Edit profile{% endtrans %}</a>
                    <a class="dropdown-item" href="{{ url_for('prefs', 'notification') }}">{% trans %}Notifications{% endtrans %}</a>
                    <div class="dropdown-divider"></div>
                    {# Logout button #}
                    <button type="submit" class="dropdown-item">{% trans %}Logout{% endtrans %}</button>
                  </div>
                </form>
        {# Logout button #}
        <form action="{{ url_for('logout') }}" method="post">
          <button type="submit" class="btn btn-link btn-sm btn-block text-center">
            {% trans %}Login with a different account{% endtrans %}
          </button>
        </form>
        self.getPage("/logout", method="POST")
            if request.method != 'POST':
                raise cherrypy.HTTPError(405)