CVE-2021-43782: Improper Neutralization of Special Elements in Output Used by a Downstream Component ('Injection') (CWE-74)
value: Tuleap is a Libre and Open Source tool for end to end traceability of application and system developments. This is a follow up to GHSA-887w-pv2r-x8pm/CVE-2021-41276, the initial fix was incomplete. Tuleap does not sanitize properly the search filter built from the ldap_id attribute of a user during the daily synchronization. A malicious user could force accounts to be suspended or take over another account by forcing the update of the ldap_uid attribute. Note that the malicious user either need to have site administrator capability on the Tuleap instance or be an LDAP operator with the capability to create/modify account. The Tuleap instance needs to have the LDAP plugin activated and enabled for this issue to be exploitable. The following versions contain the fix: Tuleap Community Edition 13.2.99.83, Tuleap Enterprise Edition 13.1-6, and Tuleap Enterprise Edition 13.2-4.
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: 7.2
Published: 2021-12-15T20:15Z
Repository: https://github.com/Enalean/tuleap
Language: PHP
Files changed: plugins/ldap/include/LDAP_ProjectGroupDao.class.php, plugins/ldap/include/LDAP_UserManager.class.php, src/common/DB/Compat/Legacy2018/CompatPDODataAccess.php, src/common/DB/Compat/Legacy2018/LegacyDataAccessInterface.php
Commit date: 2021-11-23 15:14:54 +0100
Commit message: request #24168: Indirect LDAP injection via the ldap_id attribute of a user when checking if it exists

This is a follow up to git #tuleap/stable/bd47f29847fcd6a68d359bc8aefb8749bb8a1b7c the initial fix was incomplete.
Issue was identified thanks to Psalm taint analysis.

Change-Id: I695be7d006e0cabdb9d4804f62772b0d88f3ffc0

Diff: diff --git a/plugins/ldap/include/LDAP_ProjectGroupDao.class.php b/plugins/ldap/include/LDAP_ProjectGroupDao.class.php
index 2eaf6de6d2d..6dcd6a8cbe4 100644
--- a/plugins/ldap/include/LDAP_ProjectGroupDao.class.php
+++ b/plugins/ldap/include/LDAP_ProjectGroupDao.class.php
@@ -146,10 +146,13 @@ class LDAP_ProjectGroupDao extends DataAccessObject
                 LIMIT 1";
 
         return count($this->retrieve($sql)) > 0;
     }
 
+    /**
+     * @psalm-taint-escape ldap DN is stored directly in plugin_ldap_project_group.ldap_group_dn, so we are forced to remove the taint
+     */
     public function getSynchronizedProjects()
     {
         $auto_synchronized_value = $this->da->quoteSmart(LDAP_GroupManager::AUTO_SYNCHRONIZATION);
 
         $sql = "SELECT *
diff --git a/plugins/ldap/include/LDAP_UserManager.class.php b/plugins/ldap/include/LDAP_UserManager.class.php
index f9f77ced5de..5ad324eae0a 100644
--- a/plugins/ldap/include/LDAP_UserManager.class.php
+++ b/plugins/ldap/include/LDAP_UserManager.class.php
@@ -503,11 +503,11 @@ class LDAP_UserManager
      * @return bool
      *
      */
     public function isUserDeletedFromLdap($row)
     {
-        $ldap_query = $this->ldap->getLDAPParam('eduid') . '=' . $row['ldap_id'];
+        $ldap_query = $this->ldap->getLDAPParam('eduid') . '=' . ldap_escape($row['ldap_id'], '', LDAP_ESCAPE_FILTER);
         $attributes = $this->user_sync->getSyncAttributes($this->ldap);
         $ldapSearch = false;
 
         foreach (explode(';', $this->ldap->getLDAPParam('people_dn') ?? '') as $people_dn) {
             $ldapSearch = $this->ldap->search($people_dn, $ldap_query, LDAP::SCOPE_ONELEVEL, $attributes);
diff --git a/src/common/DB/Compat/Legacy2018/CompatPDODataAccess.php b/src/common/DB/Compat/Legacy2018/CompatPDODataAccess.php
index 34d1bdde867..bf89909633c 100644
--- a/src/common/DB/Compat/Legacy2018/CompatPDODataAccess.php
+++ b/src/common/DB/Compat/Legacy2018/CompatPDODataAccess.php
@@ -47,10 +47,11 @@ final class CompatPDODataAccess implements LegacyDataAccessInterface
      * @deprecated
      * @return object MySQLDataAccessResultInterface
      *
      * @psalm-taint-sink sql $sql
      * @psalm-taint-sink sql $params
+     * @psalm-taint-source ldap
      */
     public function query($sql, $params = [])
     {
         if (! empty($params)) {
             $args = [];
@@ -307,10 +308,11 @@ final class CompatPDODataAccess implements LegacyDataAccessInterface
      * @param CompatPDODataAccessResult|false $result The result resource that is being evaluated. This result comes from a call to query().
      *
      * @deprecated
      *
      * @return array Returns an associative array of strings that corresponds to the fetched row, or FALSE if there are no more rows.
+     * @psalm-taint-source ldap
      */
     public function fetch($result)
     {
         if (! $result) {
             return false;
@@ -328,10 +330,11 @@ final class CompatPDODataAccess implements LegacyDataAccessInterface
      *
      * @deprecated since version 4.0
      * @param type $result
      *
      * @return type
+     * @psalm-taint-source ldap
      */
     public function fetchArray($result)
     {
         return $this->fetch($result);
     }
diff --git a/src/common/DB/Compat/Legacy2018/LegacyDataAccessInterface.php b/src/common/DB/Compat/Legacy2018/LegacyDataAccessInterface.php
index 00e584b8c77..da571ecd0df 100644
--- a/src/common/DB/Compat/Legacy2018/LegacyDataAccessInterface.php
+++ b/src/common/DB/Compat/Legacy2018/LegacyDataAccessInterface.php
@@ -31,10 +31,11 @@ interface LegacyDataAccessInterface
      * @deprecated
      * @return object MySQLDataAccessResultInterface
      *
      * @psalm-taint-sink sql $sql
      * @psalm-taint-sink sql $params
+     * @psalm-taint-source ldap
      */
     public function query($sql, $params = []);
 
     /**
      * Return ID generated from the previous INSERT operation.
@@ -165,10 +166,11 @@ interface LegacyDataAccessInterface
      * @param resource $result The result resource that is being evaluated. This result comes from a call to query().
      *
      * @deprecated
      *
      * @return array Returns an associative array of strings that corresponds to the fetched row, or FALSE if there are no more rows.
+     * @psalm-taint-source ldap
      */
     public function fetch($result);
 
     /**
      * Backward compatibility with database.php

Vulnerable code:         $ldap_query = $this->ldap->getLDAPParam('eduid') . '=' . $row['ldap_id'];
Fixed code:     /**
     * @psalm-taint-escape ldap DN is stored directly in plugin_ldap_project_group.ldap_group_dn, so we are forced to remove the taint
     */
        $ldap_query = $this->ldap->getLDAPParam('eduid') . '=' . ldap_escape($row['ldap_id'], '', LDAP_ESCAPE_FILTER);
     * @psalm-taint-source ldap
     * @psalm-taint-source ldap
     * @psalm-taint-source ldap
     * @psalm-taint-source ldap
     * @psalm-taint-source ldap