CVE-2021-41155: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: Tuleap is a Free & Open Source Suite to improve management of software developments and collaboration. In affected versions Tuleap does not sanitize properly user inputs when constructing the SQL query to browse and search revisions in the CVS repositories. The following versions contain the fix: Tuleap Community Edition 11.17.99.146, Tuleap Enterprise Edition 11.17-5, Tuleap Enterprise Edition 11.16-7.
CWE description: The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data.
CVSS v3 base score: 8.8
Published: 2021-10-18T22:15Z
Repository: https://github.com/Enalean/tuleap
Language: PHP
Files changed: src/www/cvs/browse_commit.php, src/www/cvs/commit_utils.php
Commit date: 2020-08-12 12:57:32 +0200
Commit message: request #16214: SQL injection in CVS revisions browser

Also fixes a reflected XSS via the same injection point.

Change-Id: I4e4d5132e748f57db46f4b685cc18e577ed5496f

Diff: diff --git a/src/www/cvs/browse_commit.php b/src/www/cvs/browse_commit.php
index 5085ff6ee35..6e46a99b469 100644
--- a/src/www/cvs/browse_commit.php
+++ b/src/www/cvs/browse_commit.php
@@ -65,20 +65,20 @@ if ($order !== false) {
         // reset list of sort criteria
         $morder = '';
     }
 }
 
-$order_by = '';
+$order_by = [];
 if ($morder !== false) {
     if (user_isloggedin()) {
         if ($morder != user_get_preference('commit_browse_order' . $group_id)) {
             user_set_preference('commit_browse_order' . $group_id, $morder);
         }
     }
 
     if ($morder !== '') {
-        $order_by = ' ORDER BY ' . commit_criteria_list_to_query($morder);
+        $order_by = commit_criteria_list_to_query($morder);
     }
 }
 
 $pm = ProjectManager::instance();
 $project = $pm->getProject($group_id);
diff --git a/src/www/cvs/commit_utils.php b/src/www/cvs/commit_utils.php
index cd997b0dbe3..655c1481f25 100644
--- a/src/www/cvs/commit_utils.php
+++ b/src/www/cvs/commit_utils.php
@@ -389,35 +389,49 @@ function commit_add_sort_criteria($criteria_list, $order, $msort)
     }
 
     return(join(',', $arr));
 }
 
-// Transform criteria list to SQL query (+ means ascending
-// - is descending)
-function commit_criteria_list_to_query($criteria_list)
+/**
+ * @psalm-return array<array{order:"ASC"|"DESC", column: "id"|"revision"|"did"|"description"|"c_when"|"date"|"f_when"|"who"}>
+ */
+function commit_criteria_list_to_query(string $criteria_list): array
 {
-    $criteria_list = str_replace('>', ' ASC', $criteria_list);
-    $criteria_list = str_replace('<', ' DESC', $criteria_list);
-
-    return $criteria_list;
+    $order_list = [];
+    foreach (explode(',', $criteria_list) as $criteria) {
+        if (preg_match('/^(?<column>[a-z]+)(?<order>[<>]?)$/', $criteria, $matches) === 1) {
+            $column = $matches['column'];
+            if (! in_array($column, ['id', 'revision', 'did', 'description', 'c_when', 'date', 'f_when', 'who'], true)) {
+                continue;
+            }
+            $order = 'ASC';
+            if ($matches['order'] === '<') {
+                $order = 'DESC';
+            }
+            $order_list[] = ['order' => $order, 'column' => $column];
+        }
+    }
+    return $order_list;
 }
 
 // Transform criteria list to readable text statement
 // $url must not contain the morder parameter
 function commit_criteria_list_to_text($criteria_list, $url)
 {
     if ($criteria_list) {
         $arr    = explode(',', $criteria_list);
         $morder = '';
 
+        $purifier = Codendi_HTMLPurifier::instance();
+
         foreach ($arr as $crit) {
             $morder .= ($morder ? "," . $crit : $crit);
             $attr = str_replace('>', '', $crit);
             $attr = str_replace('<', '', $attr);
 
-            $arr_text[] = '<a href="' . $url . '&morder=' . $morder . '#results">' .
-            commit_field_get_label($attr) . '</a><img src="' . util_get_dir_image_theme() .
+            $arr_text[] = '<a href="' . $url . '&morder=' . $purifier->purify(urlencode($morder)) . '#results">' .
+            $purifier->purify(commit_field_get_label($attr)) . '</a><img src="' . util_get_dir_image_theme() .
             ((substr($crit, -1) == '<') ? 'dn' : 'up') .
             '_arrow.png" border="0">';
         }
     }
 
@@ -707,11 +721,14 @@ function get_group_id_from_repository($repository)
     }
 
     return $project->getID();
 }
 
-function cvs_get_revisions(&$project, $offset, $chunksz, $_tag = 100, $_branch = 100, $_commit_id = '', $_commiter = 100, $_srch = '', $order_by = '', $pv = 0)
+/**
+ * @psalm-param array<array{order:"ASC"|"DESC", column: "id"|"revision"|"did"|"description"|"c_when"|"date"|"f_when"|"who"}> $order_by
+ */
+function cvs_get_revisions($project, $offset, $chunksz, $_tag = 100, $_branch = 100, $_commit_id = '', $_commiter = 100, $_srch = '', array $order_by = [], $pv = 0)
 {
     //if status selected, and more to where clause
     if ($_branch != 100) {
         //for open tasks, add status=100 to make sure we show all
         $branch_str = "AND cvs_checkins.branchid=" . db_ei($_branch);
@@ -775,18 +792,30 @@ function cvs_get_revisions(&$project, $offset, $chunksz, $_tag = 100, $_branch =
     $limit = '';
     if (! $pv) {
         $limit = " LIMIT $offset,$chunksz";
     }
 
-    if (! $order_by) {
-        $order_by = " ORDER BY id desc, f_when desc ";
+    if (empty($order_by)) {
+        $order_by_sql = ' ORDER BY id desc, f_when desc ';
+    } else {
+        $order_by_sql = ' ORDER BY ';
+        $order_by_sql .= implode(
+            ',',
+            array_map(
+                static function (array $order_by_row) {
+                    return $order_by_row['column'] . ' ' . $order_by_row['order'];
+                },
+                $order_by
+            )
+        );
+        $order_by_sql .= ' ';
     }
 
     $sql = $select .
     $from .
     $where .
-    $order_by .
+    $order_by_sql .
     $limit;
 
     $result = db_query($sql);
 
     /* expensive way to have total rows number didn'get a cheaper one */

Vulnerable code: $order_by = '';
        $order_by = ' ORDER BY ' . commit_criteria_list_to_query($morder);
// Transform criteria list to SQL query (+ means ascending
// - is descending)
function commit_criteria_list_to_query($criteria_list)
    $criteria_list = str_replace('>', ' ASC', $criteria_list);
    $criteria_list = str_replace('<', ' DESC', $criteria_list);

    return $criteria_list;
            $arr_text[] = '<a href="' . $url . '&morder=' . $morder . '#results">' .
            commit_field_get_label($attr) . '</a><img src="' . util_get_dir_image_theme() .
function cvs_get_revisions(&$project, $offset, $chunksz, $_tag = 100, $_branch = 100, $_commit_id = '', $_commiter = 100, $_srch = '', $order_by = '', $pv = 0)
    if (! $order_by) {
        $order_by = " ORDER BY id desc, f_when desc ";
    $order_by .
Fixed code: $order_by = [];
        $order_by = commit_criteria_list_to_query($morder);
/**
 * @psalm-return array<array{order:"ASC"|"DESC", column: "id"|"revision"|"did"|"description"|"c_when"|"date"|"f_when"|"who"}>
 */
function commit_criteria_list_to_query(string $criteria_list): array
    $order_list = [];
    foreach (explode(',', $criteria_list) as $criteria) {
        if (preg_match('/^(?<column>[a-z]+)(?<order>[<>]?)$/', $criteria, $matches) === 1) {
            $column = $matches['column'];
            if (! in_array($column, ['id', 'revision', 'did', 'description', 'c_when', 'date', 'f_when', 'who'], true)) {
                continue;
            }
            $order = 'ASC';
            if ($matches['order'] === '<') {
                $order = 'DESC';
            }
            $order_list[] = ['order' => $order, 'column' => $column];
        }
    }
    return $order_list;
        $purifier = Codendi_HTMLPurifier::instance();

            $arr_text[] = '<a href="' . $url . '&morder=' . $purifier->purify(urlencode($morder)) . '#results">' .
            $purifier->purify(commit_field_get_label($attr)) . '</a><img src="' . util_get_dir_image_theme() .
/**
 * @psalm-param array<array{order:"ASC"|"DESC", column: "id"|"revision"|"did"|"description"|"c_when"|"date"|"f_when"|"who"}> $order_by
 */
function cvs_get_revisions($project, $offset, $chunksz, $_tag = 100, $_branch = 100, $_commit_id = '', $_commiter = 100, $_srch = '', array $order_by = [], $pv = 0)
    if (empty($order_by)) {
        $order_by_sql = ' ORDER BY id desc, f_when desc ';
    } else {
        $order_by_sql = ' ORDER BY ';
        $order_by_sql .= implode(
            ',',
            array_map(
                static function (array $order_by_row) {
                    return $order_by_row['column'] . ' ' . $order_by_row['order'];
                },
                $order_by
            )
        );
        $order_by_sql .= ' ';
    $order_by_sql .