From 64e77561eba9f8233199c2962b3497ed7294a7d2 Mon Sep 17 00:00:00 2001
From: Thomas Gerbet <thomas.gerbet@enalean.com>
Date: Tue, 23 Nov 2021 15:14:54 +0100
Subject: [PATCH] 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
---
 plugins/ldap/include/LDAP_ProjectGroupDao.class.php           | 3 +++
 plugins/ldap/include/LDAP_UserManager.class.php               | 2 +-
 src/common/DB/Compat/Legacy2018/CompatPDODataAccess.php       | 3 +++
 src/common/DB/Compat/Legacy2018/LegacyDataAccessInterface.php | 2 ++
 4 files changed, 9 insertions(+), 1 deletion(-)

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
@@ -148,6 +148,9 @@ public function doesProjectBindingKeepUsers($project_id)
         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);
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
@@ -505,7 +505,7 @@ public function checkThreshold($nbr_users_to_suspend, $nbr_all_users)
      */
     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;
 
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
@@ -49,6 +49,7 @@ public function __construct(DBConnection $db_connection)
      *
      * @psalm-taint-sink sql $sql
      * @psalm-taint-sink sql $params
+     * @psalm-taint-source ldap
      */
     public function query($sql, $params = [])
     {
@@ -309,6 +310,7 @@ public function numRows($result)
      * @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)
     {
@@ -330,6 +332,7 @@ public function fetch($result)
      * @param type $result
      *
      * @return type
+     * @psalm-taint-source ldap
      */
     public function fetchArray($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
@@ -33,6 +33,7 @@ interface LegacyDataAccessInterface
      *
      * @psalm-taint-sink sql $sql
      * @psalm-taint-sink sql $params
+     * @psalm-taint-source ldap
      */
     public function query($sql, $params = []);
 
@@ -167,6 +168,7 @@ public function numRows($result);
      * @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);