CVE-2020-27602: Improper Neutralization of Special Elements in Output Used by a Downstream Component ('Injection') (CWE-74)
value: BigBlueButton before 2.2.7 does not have a protection mechanism for separator injection in meetingId, userId, and authToken.
CWE description: The product constructs all or part of a command, data structure, or record using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify how it is parsed or interpreted when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2022-09-29T03:15Z
Repository: https://github.com/bigbluebutton/bigbluebutton
Language: JavaScript
Files changed: bigbluebutton-html5/imports/api/users/server/handlers/validateAuthToken.js, bigbluebutton-html5/imports/api/users/server/methods/validateAuthToken.js, bigbluebutton-html5/imports/api/users/server/store/pendingAuthentications.js
Commit date: 2020-04-29 00:33:45 -0300
Commit message: Refactor connection definition of userId to wait for validateAuthToken

Diff: diff --git a/bigbluebutton-html5/imports/api/users/server/handlers/validateAuthToken.js b/bigbluebutton-html5/imports/api/users/server/handlers/validateAuthToken.js
index f8a04154b1..dae1bf3b4a 100644
--- a/bigbluebutton-html5/imports/api/users/server/handlers/validateAuthToken.js
+++ b/bigbluebutton-html5/imports/api/users/server/handlers/validateAuthToken.js
@@ -1,25 +1,74 @@
 import { check } from 'meteor/check';
 import Logger from '/imports/startup/server/logger';
 import Users from '/imports/api/users';
 import userJoin from './userJoin';
+import pendingAuthenticationsStore from '../store/pendingAuthentications';
+import createDummyUser from '../modifiers/createDummyUser';
+import setConnectionIdAndAuthToken from '../modifiers/setConnectionIdAndAuthToken';
 
 const clearOtherSessions = (sessionUserId, current = false) => {
   const serverSessions = Meteor.server.sessions;
   Object.keys(serverSessions)
     .filter(i => serverSessions[i].userId === sessionUserId)
     .filter(i => i !== current)
     .forEach(i => serverSessions[i].close());
 };
 
 export default function handleValidateAuthToken({ body }, meetingId) {
-  const { userId, valid, waitForApproval } = body;
+  const { userId, valid, authToken, waitForApproval } = body;
 
   check(userId, String);
+  check(authToken, String);
   check(valid, Boolean);
   check(waitForApproval, Boolean);
 
+  const pendingAuths = pendingAuthenticationsStore.take(meetingId, userId, authToken);
+
+  if(!valid) {
+    pendingAuths.forEach ( 
+      pendingAuth => {
+        try {
+          const {methodInvocationObject} = pendingAuth;
+          const connectionId = methodInvocationObject.connection.id;
+
+          methodInvocationObject.connection.close();
+          Logger.info(`Closed connection ${connectionId} due to invalid auth token.`);
+        } catch (e) {
+          Logger.error(`Error closing socket for meetingId '${meetingId}', userId '${userId}', authToken ${authToken}`);
+        }
+      }
+    );
+    
+    return;
+  }
+
+  if(valid) {
+    // Define user ID on connections
+    pendingAuths.forEach ( 
+        pendingAuth => {
+            const {methodInvocationObject} = pendingAuth;
+
+            /* Logic migrated from validateAuthToken method ( postponed to only run in case of success response ) - Begin */
+            const sessionId = `${meetingId}--${userId}`;
+            methodInvocationObject.setUserId(sessionId);
+
+            const User = Users.findOne({
+                meetingId,
+                userId: userId,
+            });
+        
+            if (!User) {
+                createDummyUser(meetingId, userId, authToken);
+            }
+        
+            setConnectionIdAndAuthToken(meetingId, userId, methodInvocationObject.connection.id, authToken);
+            /* End of logic migrated from validateAuthToken */
+        }
+    );
+  }
+
   const selector = {
     meetingId,
     userId,
     clientType: 'HTML5',
   };
diff --git a/bigbluebutton-html5/imports/api/users/server/methods/validateAuthToken.js b/bigbluebutton-html5/imports/api/users/server/methods/validateAuthToken.js
index e01b6ad348..0f01fb0c2d 100644
--- a/bigbluebutton-html5/imports/api/users/server/methods/validateAuthToken.js
+++ b/bigbluebutton-html5/imports/api/users/server/methods/validateAuthToken.js
@@ -1,35 +1,22 @@
 import { Meteor } from 'meteor/meteor';
 import RedisPubSub from '/imports/startup/server/redis';
 import Logger from '/imports/startup/server/logger';
-import Users from '/imports/api/users';
-import createDummyUser from '../modifiers/createDummyUser';
-import setConnectionIdAndAuthToken from '../modifiers/setConnectionIdAndAuthToken';
+import pendingAuthenticationsStore from '../store/pendingAuthentications';
 
 export default function validateAuthToken(meetingId, requesterUserId, requesterToken) {
   const REDIS_CONFIG = Meteor.settings.private.redis;
   const CHANNEL = REDIS_CONFIG.channels.toAkkaApps;
   const EVENT_NAME = 'ValidateAuthTokenReqMsg';
 
-  const sessionId = `${meetingId}--${requesterUserId}`;
-  this.setUserId(sessionId);
-
-  const User = Users.findOne({
-    meetingId,
-    userId: requesterUserId,
-  });
-
-  if (!User) {
-    createDummyUser(meetingId, requesterUserId, requesterToken);
-  }
-
-  setConnectionIdAndAuthToken(meetingId, requesterUserId, this.connection.id, requesterToken);
+  // Store reference of methodInvocationObject ( to postpone the connection userId definition )
+  pendingAuthenticationsStore.add(meetingId, requesterUserId, requesterToken, this);
 
   const payload = {
     userId: requesterUserId,
     authToken: requesterToken,
   };
 
-  Logger.info(`User '${requesterUserId}' is trying to validate auth token for meeting '${meetingId}'`);
+  Logger.info(`User '${requesterUserId}' is trying to validate auth token for meeting '${meetingId}' from connection '${this.connection.id}'`);
 
   return RedisPubSub.publishUserMessage(CHANNEL, EVENT_NAME, meetingId, requesterUserId, payload);
 }
diff --git a/bigbluebutton-html5/imports/api/users/server/store/pendingAuthentications.js b/bigbluebutton-html5/imports/api/users/server/store/pendingAuthentications.js
new file mode 100644
index 0000000000..c181ac8f95
--- /dev/null
+++ b/bigbluebutton-html5/imports/api/users/server/store/pendingAuthentications.js
@@ -0,0 +1,43 @@
+import Logger from '/imports/startup/server/logger';
+
+class PendingAuthentitcations {
+    constructor () {
+        Logger.debug("PendingAuthentitcations :: constructor");
+        this.store = [];
+    }
+
+    generateKey (meetingId, userId, authToken) {
+        // Protect against separator injection
+        meetingId = meetingId.replace(/ /g, '');
+        userId    = userId.replace(/ /g, '');
+        authToken = authToken.replace(/ /g, '');
+
+        // Space separated key
+        return '${meetingId} ${userId} ${authToken}';
+    }
+
+    add (meetingId, userId, authToken, methodInvocationObject) {
+        Logger.debug("PendingAuthentitcations :: add", {meetingId, userId, authToken});
+        this.store.push({
+            key: this.generateKey(meetingId, userId, authToken),
+            meetingId, userId, authToken, methodInvocationObject
+        });
+    }
+
+    take (meetingId, userId, authToken) {
+        Logger.debug("PendingAuthentitcations :: take", {meetingId, userId, authToken});
+        const key = this.generateKey(meetingId, userId, authToken);
+        
+        // find matches
+        const matches = this.store.filter( e => e.key === key );
+        
+        // remove matches (if any)
+        if(matches.length) { 
+            this.store = this.store.filter( e => e.key !== key ) ;
+        }
+
+        // return matches
+        return matches;
+    }
+}
+export default new PendingAuthentitcations();
\ No newline at end of file

Vulnerable code:   const { userId, valid, waitForApproval } = body;
import Users from '/imports/api/users';
import createDummyUser from '../modifiers/createDummyUser';
import setConnectionIdAndAuthToken from '../modifiers/setConnectionIdAndAuthToken';
  const sessionId = `${meetingId}--${requesterUserId}`;
  this.setUserId(sessionId);

  const User = Users.findOne({
    meetingId,
    userId: requesterUserId,
  });

  if (!User) {
    createDummyUser(meetingId, requesterUserId, requesterToken);
  }

  setConnectionIdAndAuthToken(meetingId, requesterUserId, this.connection.id, requesterToken);
  Logger.info(`User '${requesterUserId}' is trying to validate auth token for meeting '${meetingId}'`);
Fixed code: import pendingAuthenticationsStore from '../store/pendingAuthentications';
import createDummyUser from '../modifiers/createDummyUser';
import setConnectionIdAndAuthToken from '../modifiers/setConnectionIdAndAuthToken';
  const { userId, valid, authToken, waitForApproval } = body;
  check(authToken, String);
  const pendingAuths = pendingAuthenticationsStore.take(meetingId, userId, authToken);

  if(!valid) {
    pendingAuths.forEach ( 
      pendingAuth => {
        try {
          const {methodInvocationObject} = pendingAuth;
          const connectionId = methodInvocationObject.connection.id;

          methodInvocationObject.connection.close();
          Logger.info(`Closed connection ${connectionId} due to invalid auth token.`);
        } catch (e) {
          Logger.error(`Error closing socket for meetingId '${meetingId}', userId '${userId}', authToken ${authToken}`);
        }
      }
    );
    
    return;
  }

  if(valid) {
    // Define user ID on connections
    pendingAuths.forEach ( 
        pendingAuth => {
            const {methodInvocationObject} = pendingAuth;

            /* Logic migrated from validateAuthToken method ( postponed to only run in case of success response ) - Begin */
            const sessionId = `${meetingId}--${userId}`;
            methodInvocationObject.setUserId(sessionId);

            const User = Users.findOne({
                meetingId,
                userId: userId,
            });
        
            if (!User) {
                createDummyUser(meetingId, userId, authToken);
            }
        
            setConnectionIdAndAuthToken(meetingId, userId, methodInvocationObject.connection.id, authToken);
            /* End of logic migrated from validateAuthToken */
        }
    );
  }

import pendingAuthenticationsStore from '../store/pendingAuthentications';
  // Store reference of methodInvocationObject ( to postpone the connection userId definition )
  pendingAuthenticationsStore.add(meetingId, requesterUserId, requesterToken, this);
  Logger.info(`User '${requesterUserId}' is trying to validate auth token for meeting '${meetingId}' from connection '${this.connection.id}'`);
import Logger from '/imports/startup/server/logger';

class PendingAuthentitcations {
    constructor () {
        Logger.debug("PendingAuthentitcations :: constructor");
        this.store = [];
    }

    generateKey (meetingId, userId, authToken) {
        // Protect against separator injection
        meetingId = meetingId.replace(/ /g, '');
        userId    = userId.replace(/ /g, '');
        authToken = authToken.replace(/ /g, '');

        // Space separated key
        return '${meetingId} ${userId} ${authToken}';
    }

    add (meetingId, userId, authToken, methodInvocationObject) {
        Logger.debug("PendingAuthentitcations :: add", {meetingId, userId, authToken});
        this.store.push({
            key: this.generateKey(meetingId, userId, authToken),
            meetingId, userId, authToken, methodInvocationObject
        });
    }

    take (meetingId, userId, authToken) {
        Logger.debug("PendingAuthentitcations :: take", {meetingId, userId, authToken});
        const key = this.generateKey(meetingId, userId, authToken);
        
        // find matches
        const matches = this.store.filter( e => e.key === key );
        
        // remove matches (if any)
        if(matches.length) { 
            this.store = this.store.filter( e => e.key !== key ) ;
        }

        // return matches
        return matches;
    }
}
export default new PendingAuthentitcations();