CVE-2020-28954: Improper Encoding or Escaping of Output (CWE-116)
value: web/controllers/ApiController.groovy in BigBlueButton before 2.2.29 lacks certain parameter sanitization, as demonstrated by accepting control characters in a user name.
CWE description: The product prepares a structured message for communication with another component, but encoding or escaping of the data is either missing or done incorrectly. As a result, the intended structure of the message is not preserved.
CVSS v3 base score: 5.3
Published: 2020-11-19T22:15Z
Repository: https://github.com/bigbluebutton/bigbluebutton
Language: Other
Files changed: bigbluebutton-web/grails-app/controllers/org/bigbluebutton/web/controllers/ApiController.groovy
Commit date: 2020-11-13 06:54:32 +0000
Commit message: Sanitize all received parameters

Diff: diff --git a/bigbluebutton-web/grails-app/controllers/org/bigbluebutton/web/controllers/ApiController.groovy b/bigbluebutton-web/grails-app/controllers/org/bigbluebutton/web/controllers/ApiController.groovy
index 38610fde2cd..776b7a73bd9 100755
--- a/bigbluebutton-web/grails-app/controllers/org/bigbluebutton/web/controllers/ApiController.groovy
+++ b/bigbluebutton-web/grails-app/controllers/org/bigbluebutton/web/controllers/ApiController.groovy
@@ -91,10 +91,15 @@ class ApiController {
   def create = {
     String API_CALL = 'create'
     log.debug CONTROLLER_NAME + "#${API_CALL}"
     log.debug request.getParameterMap().toMapString()
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     // BEGIN - backward compatibility
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
@@ -173,10 +178,15 @@ class ApiController {
   def join = {
     String API_CALL = 'join'
     log.debug CONTROLLER_NAME + "#${API_CALL}"
     ApiErrors errors = new ApiErrors()
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     // BEGIN - backward compatibility
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check", REDIRECT_RESPONSE)
       return
     }
@@ -242,13 +252,10 @@ class ApiController {
       joinViaHtml5 = Boolean.parseBoolean(params.joinViaHtml5)
     }
 
     // Do we have a name for the user joining? If none, complain.
     if (!StringUtils.isEmpty(params.fullName)) {
-      params.fullName = StringUtils.strip(params.fullName);
-      // remove control characters ( sanitize )
-      params.fullName = params.fullName.replaceAll("\\p{Cntrl}", "");
       if (StringUtils.isEmpty(params.fullName)) {
         errors.missingParamError("fullName");
       }
     } else {
       errors.missingParamError("fullName");
@@ -556,10 +563,15 @@ class ApiController {
    *******************************************/
   def isMeetingRunning = {
     String API_CALL = 'isMeetingRunning'
     log.debug CONTROLLER_NAME + "#${API_CALL}"
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     // BEGIN - backward compatibility
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
@@ -632,13 +644,17 @@ class ApiController {
   /************************************
    * END API
    ************************************/
   def end = {
     String API_CALL = "end"
-
     log.debug CONTROLLER_NAME + "#${API_CALL}"
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     // BEGIN - backward compatibility
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
@@ -757,10 +773,15 @@ class ApiController {
    *****************************************/
   def getMeetingInfo = {
     String API_CALL = "getMeetingInfo"
     log.debug CONTROLLER_NAME + "#${API_CALL}"
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     // BEGIN - backward compatibility
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
@@ -840,10 +861,15 @@ class ApiController {
    ************************************/
   def getMeetingsHandler = {
     String API_CALL = "getMeetings"
     log.debug CONTROLLER_NAME + "#${API_CALL}"
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     // BEGIN - backward compatibility
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
@@ -898,10 +924,15 @@ class ApiController {
    ************************************/
   def getSessionsHandler = {
     String API_CALL = "getSessions"
     log.debug CONTROLLER_NAME + "#${API_CALL}"
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     // BEGIN - backward compatibility
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
@@ -973,10 +1004,15 @@ class ApiController {
    ***********************************************/
   def setPollXML = {
     String API_CALL = "setPollXML"
     log.debug CONTROLLER_NAME + "#${API_CALL}"
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
 
@@ -1059,10 +1095,15 @@ class ApiController {
    ***********************************************/
   def setConfigXML = {
     String API_CALL = "setConfigXML"
     log.debug CONTROLLER_NAME + "#${API_CALL}"
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
 
@@ -1138,10 +1179,15 @@ class ApiController {
   def getDefaultConfigXML = {
 
     String API_CALL = "getDefaultConfigXML"
     ApiErrors errors = new ApiErrors();
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     // BEGIN - backward compatibility
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
@@ -1177,10 +1223,15 @@ class ApiController {
 
   def configXML = {
     String API_CALL = 'configXML'
     log.debug CONTROLLER_NAME + "#${API_CALL}"
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     String logoutUrl = paramsProcessorUtil.getDefaultLogoutUrl()
     boolean reject = false
     String sessionToken = sanitizeSessionToken(params.sessionToken)
     if (!hasValidSession(sessionToken)) {
       response.addHeader("Cache-Control", "no-cache")
@@ -1224,10 +1275,16 @@ class ApiController {
    * GUEST WAIT API
    *********************************************/
   def guestWaitHandler = {
     String API_CALL = 'guestWait'
     log.debug CONTROLLER_NAME + "#${API_CALL}"
+
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     ApiErrors errors = new ApiErrors()
     boolean reject = false;
     String sessionToken = sanitizeSessionToken(params.sessionToken)
 
     UserSession us = getUserSession(sessionToken);
@@ -1367,10 +1424,18 @@ class ApiController {
 
   /***********************************************
    * ENTER API
    ***********************************************/
   def enter = {
+    String API_CALL = 'enter'
+    log.debug CONTROLLER_NAME + "#${API_CALL}"
+
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     boolean reject = false;
 
     String sessionToken = sanitizeSessionToken(params.sessionToken)
     UserSession us = getUserSession(sessionToken);
     Meeting meeting = null;
@@ -1509,10 +1574,18 @@ class ApiController {
 
   /***********************************************
    * STUN/TURN API
    ***********************************************/
   def stuns = {
+    String API_CALL = 'stuns'
+    log.debug CONTROLLER_NAME + "#${API_CALL}"
+
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     boolean reject = false;
 
     String sessionToken = sanitizeSessionToken(params.sessionToken)
     UserSession us = getUserSession(sessionToken);
     Meeting meeting = null;
@@ -1580,10 +1653,17 @@ class ApiController {
 
   /*************************************************
    * SIGNOUT API
    *************************************************/
   def signOut = {
+    String API_CALL = 'signOut'
+    log.debug CONTROLLER_NAME + "#${API_CALL}"
+
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
 
     String sessionToken = sanitizeSessionToken(params.sessionToken)
 
     Meeting meeting = null;
 
@@ -1626,10 +1706,15 @@ class ApiController {
    ******************************************************/
   def getRecordingsHandler = {
     String API_CALL = "getRecordings"
     log.debug CONTROLLER_NAME + "#${API_CALL}"
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     // BEGIN - backward compatibility
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
@@ -1700,10 +1785,15 @@ class ApiController {
    ******************************************************/
   def publishRecordings = {
     String API_CALL = "publishRecordings"
     log.debug CONTROLLER_NAME + "#${API_CALL}"
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     // BEGIN - backward compatibility
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
@@ -1781,10 +1871,15 @@ class ApiController {
    ******************************************************/
   def deleteRecordings = {
     String API_CALL = "deleteRecordings"
     log.debug CONTROLLER_NAME + "#${API_CALL}"
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     // BEGIN - backward compatibility
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
@@ -1851,10 +1946,15 @@ class ApiController {
    ******************************************************/
   def updateRecordingsHandler = {
     String API_CALL = "updateRecordings"
     log.debug CONTROLLER_NAME + "#${API_CALL}"
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     // BEGIN - backward compatibility
     if (StringUtils.isEmpty(params.checksum)) {
       invalid("checksumError", "You did not pass the checksum security check")
       return
     }
@@ -1922,10 +2022,15 @@ class ApiController {
   }
 
   def uploadDocuments(conf) { //
     log.debug("ApiController#uploadDocuments(${conf.getInternalId()})");
 
+    //sanitizeInput
+    params.each {
+      key, value -> params[key] = sanitizeInput(value)
+    }
+
     String requestBody = request.inputStream == null ? null : request.inputStream.text;
     requestBody = StringUtils.isEmpty(requestBody) ? null : requestBody;
 
     if (requestBody == null) {
       downloadAndProcessDocument(presentationService.defaultUploadedPresentation, conf.getInternalId(), true /* default presentation */, '');
@@ -2110,10 +2215,20 @@ class ApiController {
     }
 
     return us
   }
 
+  private def sanitizeInput (input) {
+    if(input == null)
+      return
+
+    if(!("java.lang.String".equals(input.getClass().getName())))
+      return input
+
+    StringUtils.strip(input.replaceAll("\\p{Cntrl}", ""));
+  }
+
   def sanitizeSessionToken(param) {
     if (param == null) {
       log.info("sanitizeSessionToken: token is null")
       return null
     }

Vulnerable code:       params.fullName = StringUtils.strip(params.fullName);
      // remove control characters ( sanitize )
      params.fullName = params.fullName.replaceAll("\\p{Cntrl}", "");

Fixed code:     //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }


    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    String API_CALL = 'enter'
    log.debug CONTROLLER_NAME + "#${API_CALL}"

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    String API_CALL = 'stuns'
    log.debug CONTROLLER_NAME + "#${API_CALL}"

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    String API_CALL = 'signOut'
    log.debug CONTROLLER_NAME + "#${API_CALL}"

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }
    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

    //sanitizeInput
    params.each {
      key, value -> params[key] = sanitizeInput(value)
    }

  private def sanitizeInput (input) {
    if(input == null)
      return

    if(!("java.lang.String".equals(input.getClass().getName())))
      return input

    StringUtils.strip(input.replaceAll("\\p{Cntrl}", ""));
  }