CVE-2023-40021: Observable Discrepancy (CWE-203)
value: Oppia is an online learning platform. When comparing a received CSRF token against the expected token, Oppia uses the string equality operator (`==`), which is not safe against timing attacks. By repeatedly submitting invalid tokens, an attacker can brute-force the expected CSRF token character by character. Once they have recovered the token, they can then submit a forged request on behalf of a logged-in user and execute privileged actions on that user's behalf. In particular the function to validate received CSRF tokens is at `oppia.core.controllers.base.CsrfTokenManager.is_csrf_token_valid`. An attacker who can lure a logged-in Oppia user to a malicious website can perform any change on Oppia that the user is authorized to do, including changing profile information; creating, deleting, and changing explorations; etc. Note that the attacker cannot change a user's login credentials. An attack would need to complete within 1 second because every second, the time used in computing the token changes. This issue has been addressed in commit `b89bf80837` which has been included in release `3.3.2-hotfix-2`. Users are advised to upgrade. There are no known workarounds for this vulnerability.
CWE description: The product behaves differently or sends different responses under different circumstances in a way that is observable to an unauthorized actor, which exposes security-relevant information about the state of the product, such as whether a particular operation was successful or not.
CVSS v3 base score: 5.3
Published: 2023-08-16T21:15Z
Repository: https://github.com/oppia/oppia
Language: Python
Files changed: core/controllers/base.py, core/controllers/base_test.py
Commit date: 2023-08-13 01:18:25 +0000
Commit message: Fix GHSA-49jp-pjc3-2532: Strengthen CSRF Protections (#18769)

* Strengthen CSRF Protections

Adds the following protections:
* A nonce to prevent collisions. This is best practice per OWASP.
* A constant-time comparison of expected and received tokens.
* HMAC-SHA256 instead of HMAC-MD5. Even though HMAC-MD5 is not broken
  yet, attacks against MD5 have been getting better, and there's no
  reason not to switch to SHA256.

* Fix base controller tests
Diff: diff --git a/core/controllers/base.py b/core/controllers/base.py
index 3ccbe4e17..e8291c7fa 100755
--- a/core/controllers/base.py
+++ b/core/controllers/base.py
@@ -900,16 +900,22 @@ class CsrfTokenManager:
         config_services.set_property(
             feconf.SYSTEM_COMMITTER_ID, CSRF_SECRET.name,
             base64.urlsafe_b64encode(os.urandom(20)))
 
     @classmethod
-    def _create_token(cls, user_id: Optional[str], issued_on: float) -> str:
+    def _create_token(
+            cls, user_id: Optional[str], issued_on: float,
+            nonce: Optional[str] = None) -> str:
         """Creates a new CSRF token.
 
         Args:
             user_id: str|None. The user_id for which the token is generated.
             issued_on: float. The timestamp at which the token was issued.
+            nonce: str|None. A token that is never reused to prevent reply
+                attacks. This argument should only be provided when validating a
+                received CSRF token, in which case the nonce in the received
+                token should be provided here.
 
         Returns:
             str. The generated CSRF token.
         """
         cls.init_csrf_secret()
@@ -921,23 +927,35 @@ class CsrfTokenManager:
             user_id = cls._USER_ID_DEFAULT
 
         # Round time to seconds.
         issued_on_str = str(int(issued_on))
 
+        # Generate a nonce (number used once) to ensure that even two
+        # consecutive calls to the same endpoint in the same second generate
+        # different tokens. Note that this nonce is just for anti-collision
+        # purposes, so it's okay that the nonce is stored in the CSRF token and
+        # therefore can be controlled by an attacker. See OWASP guidance here:
+        # https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#signed-double-submit-cookie.
+        if nonce is None:
+            nonce = base64.urlsafe_b64encode(os.urandom(20)).decode('utf-8')
+
         digester = hmac.new(
             key=CSRF_SECRET.value.encode('utf-8'),
-            digestmod='md5'
+            digestmod='sha256'
         )
         digester.update(user_id.encode('utf-8'))
         digester.update(b':')
         digester.update(issued_on_str.encode('utf-8'))
+        digester.update(b':')
+        digester.update(nonce.encode('utf-8'))
 
         digest = digester.digest()
         # The b64encode returns bytes, so we first need to decode the returned
         # bytes to string.
-        token = '%s/%s' % (
-            issued_on_str, base64.urlsafe_b64encode(digest).decode('utf-8'))
+        token = '%s/%s/%s' % (
+            issued_on_str, nonce,
+            base64.urlsafe_b64encode(digest).decode('utf-8'))
 
         return token
 
     @classmethod
     def _get_current_time(cls) -> float:
@@ -971,20 +989,24 @@ class CsrfTokenManager:
         Returns:
             bool. Whether the given CSRF token is valid.
         """
         try:
             parts = token.split('/')
-            if len(parts) != 2:
+            if len(parts) != 3:
                 return False
 
             issued_on = int(parts[0])
             age = cls._get_current_time() - issued_on
             if age > cls._CSRF_TOKEN_AGE_SECS:
                 return False
 
-            authentic_token = cls._create_token(user_id, issued_on)
-            if authentic_token == token:
+            nonce = parts[1]
+
+            authentic_token = cls._create_token(user_id, issued_on, nonce)
+            if hmac.compare_digest(
+                authentic_token.encode('utf-8'), token.encode('utf-8')
+            ):
                 return True
 
             return False
         except Exception:
             return False
diff --git a/core/controllers/base_test.py b/core/controllers/base_test.py
index df3e489b7..d622a9382 100644
--- a/core/controllers/base_test.py
+++ b/core/controllers/base_test.py
@@ -741,11 +741,11 @@ class CsrfTokenManagerTests(test_utils.GenericTestBase):
         self.assertFalse(
             base.CsrfTokenManager.is_csrf_token_valid('bad_user', token))
         self.assertFalse(
             base.CsrfTokenManager.is_csrf_token_valid(uid, 'new_token'))
         self.assertFalse(
-            base.CsrfTokenManager.is_csrf_token_valid(uid, 'new/token'))
+            base.CsrfTokenManager.is_csrf_token_valid(uid, 'a/new/token'))
 
     def test_non_default_csrf_secret_is_used(self) -> None:
         base.CsrfTokenManager.create_csrf_token('uid')
         self.assertNotEqual(base.CSRF_SECRET.value, base.DEFAULT_CSRF_SECRET)
 

Vulnerable code:     def _create_token(cls, user_id: Optional[str], issued_on: float) -> str:
            digestmod='md5'
        token = '%s/%s' % (
            issued_on_str, base64.urlsafe_b64encode(digest).decode('utf-8'))
            if len(parts) != 2:
            authentic_token = cls._create_token(user_id, issued_on)
            if authentic_token == token:
            base.CsrfTokenManager.is_csrf_token_valid(uid, 'new/token'))
Fixed code:     def _create_token(
            cls, user_id: Optional[str], issued_on: float,
            nonce: Optional[str] = None) -> str:
            nonce: str|None. A token that is never reused to prevent reply
                attacks. This argument should only be provided when validating a
                received CSRF token, in which case the nonce in the received
                token should be provided here.
        # Generate a nonce (number used once) to ensure that even two
        # consecutive calls to the same endpoint in the same second generate
        # different tokens. Note that this nonce is just for anti-collision
        # purposes, so it's okay that the nonce is stored in the CSRF token and
        # therefore can be controlled by an attacker. See OWASP guidance here:
        # https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#signed-double-submit-cookie.
        if nonce is None:
            nonce = base64.urlsafe_b64encode(os.urandom(20)).decode('utf-8')

            digestmod='sha256'
        digester.update(b':')
        digester.update(nonce.encode('utf-8'))
        token = '%s/%s/%s' % (
            issued_on_str, nonce,
            base64.urlsafe_b64encode(digest).decode('utf-8'))
            if len(parts) != 3:
            nonce = parts[1]

            authentic_token = cls._create_token(user_id, issued_on, nonce)
            if hmac.compare_digest(
                authentic_token.encode('utf-8'), token.encode('utf-8')
            ):
            base.CsrfTokenManager.is_csrf_token_valid(uid, 'a/new/token'))