CVE-2024-4286: Improper Neutralization of Special Elements used in an Expression Language Statement ('Expression Language Injection') (CWE-917)
value: Mintplex-Labs' anything-llm application is vulnerable to improper neutralization of special elements used in an expression language statement, identified in the commit id `57984fa85c31988b2eff429adfc654c46e0c342a`. The vulnerability arises from the application's handling of user modifications by managers or admins, allowing for the modification of all existing attributes of the `user` database entity without proper checks or sanitization. This flaw can be exploited to delete user threads, denying users access to their previously submitted data, or to inject fake threads and/or chat history for social engineering attacks.
CWE description: The product constructs all or part of an expression language (EL) statement in a framework such as a Java Server Page (JSP) using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended EL statement before it is executed.
Published: 2024-05-26T23:15Z
Repository: https://github.com/mintplex-labs/anything-llm
Language: JavaScript
Files changed: server/models/user.js, server/utils/AiProviders/openRouter/index.js, server/utils/PasswordRecovery/index.js
Commit date: 2024-04-26 16:46:04 -0700
Commit message: Strengthen field validations on user Updates (#1201)

* Strengthen field validations on user Updates

* update writables
Diff: diff --git a/server/models/user.js b/server/models/user.js
index c447950ca..ecb620ee4 100644
--- a/server/models/user.js
+++ b/server/models/user.js
@@ -1,9 +1,26 @@
 const prisma = require("../utils/prisma");
 const { EventLogs } = require("./eventLogs");
 
 const User = {
+  writable: [
+    // Used for generic updates so we can validate keys in request body
+    "username",
+    "password",
+    "pfpFilename",
+    "role",
+    "suspended",
+  ],
+  // validations for the above writable fields.
+  castColumnValue: function (key, value) {
+    switch (key) {
+      case "suspended":
+        return Number(Boolean(value));
+      default:
+        return String(value);
+    }
+  },
   create: async function ({ username, password, role = "default" }) {
     const passwordCheck = this.checkPasswordComplexity(password);
     if (!passwordCheck.checkedOK) {
       return { user: null, error: passwordCheck.error };
     }
@@ -40,17 +57,30 @@ const User = {
     return changes;
   },
 
   update: async function (userId, updates = {}) {
     try {
+      if (!userId) throw new Error("No user id provided for update");
       const currentUser = await prisma.users.findUnique({
         where: { id: parseInt(userId) },
       });
-      if (!currentUser) {
-        return { success: false, error: "User not found" };
-      }
+      if (!currentUser) return { success: false, error: "User not found" };
+
+      // Removes non-writable fields for generic updates
+      // and force-casts to the proper type;
+      Object.entries(updates).forEach(([key, value]) => {
+        if (this.writable.includes(key)) {
+          updates[key] = this.castColumnValue(key, value);
+          return;
+        }
+        delete updates[key];
+      });
+
+      if (Object.keys(updates).length === 0)
+        return { success: false, error: "No valid updates applied." };
 
+      // Handle password specific updates
       if (updates.hasOwnProperty("password")) {
         const passwordCheck = this.checkPasswordComplexity(updates.password);
         if (!passwordCheck.checkedOK) {
           return { success: false, error: passwordCheck.error };
         }
@@ -76,10 +106,28 @@ const User = {
       console.error(error.message);
       return { success: false, error: error.message };
     }
   },
 
+  // Explicit direct update of user object.
+  // Only use this method when directly setting a key value
+  // that takes no user input for the keys being modified.
+  _update: async function (id = null, data = {}) {
+    if (!id) throw new Error("No user id provided for update");
+
+    try {
+      const user = await prisma.users.update({
+        where: { id },
+        data,
+      });
+      return { user, message: null };
+    } catch (error) {
+      console.error(error.message);
+      return { user: null, message: error.message };
+    }
+  },
+
   get: async function (clause = {}) {
     try {
       const user = await prisma.users.findFirst({ where: clause });
       return user ? { ...user } : null;
     } catch (error) {
diff --git a/server/utils/AiProviders/openRouter/index.js b/server/utils/AiProviders/openRouter/index.js
index ac72b1679..655839dc3 100644
--- a/server/utils/AiProviders/openRouter/index.js
+++ b/server/utils/AiProviders/openRouter/index.js
@@ -294,11 +294,11 @@ class OpenRouterLLM {
           // so we need to stitch it together. You would think JSON
           // chunks would only come complete - but they don't!
           try {
             JSON.parse(message);
             validJSON = true;
-          } catch { }
+          } catch {}
 
           if (!validJSON) {
             // It can be possible that the chunk decoding is running away
             // and the message chunk fails to append due to string length.
             // In this case abort the chunk and reset so we can continue.
diff --git a/server/utils/PasswordRecovery/index.js b/server/utils/PasswordRecovery/index.js
index fbcbe5799..2383dd2c3 100644
--- a/server/utils/PasswordRecovery/index.js
+++ b/server/utils/PasswordRecovery/index.js
@@ -20,11 +20,11 @@ async function generateRecoveryCodes(userId) {
   }
 
   const { error } = await RecoveryCode.createMany(newRecoveryCodes);
   if (!!error) throw new Error(error);
 
-  const { success } = await User.update(userId, {
+  const { user: success } = await User._update(userId, {
     seen_recovery_codes: true,
   });
   if (!success) throw new Error("Failed to generate user recovery codes!");
 
   return plainTextCodes;
@@ -78,10 +78,15 @@ async function resetPassword(token, _newPassword = "", confirmPassword = "") {
   }
 
   // JOI password rules will be enforced inside .update.
   const { error } = await User.update(resetToken.user_id, {
     password: newPassword,
+  });
+
+  // seen_recovery_codes is not publicly writable
+  // so we have to do direct update here
+  await User._update(resetToken.user_id, {
     seen_recovery_codes: false,
   });
 
   if (error) return { success: false, message: error };
   await PasswordResetToken.deleteMany({ user_id: resetToken.user_id });

Vulnerable code:       if (!currentUser) {
        return { success: false, error: "User not found" };
      }
          } catch { }
  const { success } = await User.update(userId, {
Fixed code:   writable: [
    // Used for generic updates so we can validate keys in request body
    "username",
    "password",
    "pfpFilename",
    "role",
    "suspended",
  ],
  // validations for the above writable fields.
  castColumnValue: function (key, value) {
    switch (key) {
      case "suspended":
        return Number(Boolean(value));
      default:
        return String(value);
    }
  },
      if (!userId) throw new Error("No user id provided for update");
      if (!currentUser) return { success: false, error: "User not found" };

      // Removes non-writable fields for generic updates
      // and force-casts to the proper type;
      Object.entries(updates).forEach(([key, value]) => {
        if (this.writable.includes(key)) {
          updates[key] = this.castColumnValue(key, value);
          return;
        }
        delete updates[key];
      });

      if (Object.keys(updates).length === 0)
        return { success: false, error: "No valid updates applied." };
      // Handle password specific updates
  // Explicit direct update of user object.
  // Only use this method when directly setting a key value
  // that takes no user input for the keys being modified.
  _update: async function (id = null, data = {}) {
    if (!id) throw new Error("No user id provided for update");

    try {
      const user = await prisma.users.update({
        where: { id },
        data,
      });
      return { user, message: null };
    } catch (error) {
      console.error(error.message);
      return { user: null, message: error.message };
    }
  },

          } catch {}
  const { user: success } = await User._update(userId, {
  });

  // seen_recovery_codes is not publicly writable
  // so we have to do direct update here
  await User._update(resetToken.user_id, {