CVE-2021-32644: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Ampache is an open source web based audio/video streaming application and file manager. Due to a lack of input filtering versions 4.x.y are vulnerable to code injection in random.php. The attack requires user authentication to access the random.php page unless the site is running in demo mode. This issue has been resolved in 4.4.3.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2021-06-22T18:15Z
Repository: https://github.com/ampache/ampache
Language: PHP
Files changed: lib/class/search.class.php, random.php, templates/show_random.inc.php, templates/show_rules.inc.php, templates/show_search_form.inc.php
Commit date: 2021-05-27 10:20:53 +1000
Commit message: filter random/search types, code indent

Diff: diff --git a/lib/class/search.class.php b/lib/class/search.class.php
index 2c590079e..991176aaa 100644
--- a/lib/class/search.class.php
+++ b/lib/class/search.class.php
@@ -2037,24 +2037,24 @@ class Search extends playlist_object
                         "$sql_match_operator '$input' GROUP BY `song`.`id`)";
                     break;
                 case 'myplayed':
                 case 'myplayedalbum':
                 case 'myplayedartist':
-                // combine these as they all do the same thing just different tables
-                $looking      = str_replace('myplayed', '', $rule[0]);
-                $column       = ($looking == '') ? 'id' : $looking;
-                $my_type      = ($looking == '') ? 'song' : $looking;
-                $operator_sql = ((int) $sql_match_operator == 0) ? 'IS NULL' : 'IS NOT NULL';
-                // played once per user
-                $table['myplayed'] .= (!strpos((string) $table['myplayed'], "myplayed_" . $my_type . "_" . $userid)) ?
-                    "LEFT JOIN (SELECT `object_id`, `object_type`, `user` FROM `object_count` " .
-                    "WHERE `object_count`.`object_type` = '$my_type' AND `object_count`.`count_type` = 'stream' " .
-                    "AND `object_count`.`user`=$userid GROUP BY `object_id`, `object_type`, `user`) AS " .
-                    "`myplayed_" . $my_type . "_" . $userid . "` " .
-                    "ON `song`.`$column`=`myplayed_" . $my_type . "_" . $userid . "`.`object_id` " .
-                    "AND `myplayed_" . $my_type . "_" . $userid . "`.`object_type` = '$my_type' " : ' ';
-                $where[] = "`myplayed_" . $my_type . "_" . $userid . "`.`object_id` $operator_sql";
+                    // combine these as they all do the same thing just different tables
+                    $looking      = str_replace('myplayed', '', $rule[0]);
+                    $column       = ($looking == '') ? 'id' : $looking;
+                    $my_type      = ($looking == '') ? 'song' : $looking;
+                    $operator_sql = ((int) $sql_match_operator == 0) ? 'IS NULL' : 'IS NOT NULL';
+                    // played once per user
+                    $table['myplayed'] .= (!strpos((string) $table['myplayed'], "myplayed_" . $my_type . "_" . $userid)) ?
+                        "LEFT JOIN (SELECT `object_id`, `object_type`, `user` FROM `object_count` " .
+                        "WHERE `object_count`.`object_type` = '$my_type' AND `object_count`.`count_type` = 'stream' " .
+                        "AND `object_count`.`user`=$userid GROUP BY `object_id`, `object_type`, `user`) AS " .
+                        "`myplayed_" . $my_type . "_" . $userid . "` " .
+                        "ON `song`.`$column`=`myplayed_" . $my_type . "_" . $userid . "`.`object_id` " .
+                        "AND `myplayed_" . $my_type . "_" . $userid . "`.`object_type` = '$my_type' " : ' ';
+                    $where[] = "`myplayed_" . $my_type . "_" . $userid . "`.`object_id` $operator_sql";
                     break;
                 case 'bitrate':
                     $input   = $input * 1000;
                     $where[] = "`song`.`bitrate` $sql_match_operator '$input'";
                     break;
diff --git a/random.php b/random.php
index 5a6eb4084..4d01aafd0 100644
--- a/random.php
+++ b/random.php
@@ -26,11 +26,11 @@ require_once $a_root . '/lib/init.php';
 UI::show_header();
 
 // Switch on the actions
 switch ($_REQUEST['action']) {
     case 'get_advanced':
-        $object_ids = Random::advanced($_REQUEST['type'], $_POST);
+        $object_ids = Random::advanced(filter_input(INPUT_GET, 'type', FILTER_SANITIZE_SPECIAL_CHARS), $_POST);
 
         // We need to add them to the active playlist
         if (!empty($object_ids)) {
             foreach ($object_ids as $object_id) {
                 Core::get_global('user')->playlist->add_object($object_id, 'song');
diff --git a/templates/show_random.inc.php b/templates/show_random.inc.php
index b9b2f8296..21c4ef5d4 100644
--- a/templates/show_random.inc.php
+++ b/templates/show_random.inc.php
@@ -18,11 +18,11 @@
  * You should have received a copy of the GNU Affero General Public License
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  *
  */ ?>
 <?php UI::show_box_top(T_('Play Random Selection'), 'box box_random'); ?>
-<form id="random" method="post" enctype="multipart/form-data" action="<?php echo AmpConfig::get('web_path'); ?>/random.php?action=get_advanced&type=<?php echo Core::get_request('type') ? scrub_out(Core::get_request('type')) : 'song'; ?>">
+<form id="random" method="post" enctype="multipart/form-data" action="<?php echo AmpConfig::get('web_path'); ?>/random.php?action=get_advanced&type=<?php echo (string) scrub_out(filter_input(INPUT_GET, 'type', FILTER_SANITIZE_SPECIAL_CHARS)) ?: 'song'; ?>">
 <table class="tabledata">
 <tr id="search_location">
     <td><?php if ((string) filter_input(INPUT_GET, 'type', FILTER_SANITIZE_SPECIAL_CHARS) !== 'song') {
     ?><a href="<?php echo AmpConfig::get('web_path'); ?>/random.php?action=advanced&type=song"><?php echo T_('Songs'); ?></a><?php
 } else {
diff --git a/templates/show_rules.inc.php b/templates/show_rules.inc.php
index 7a84f9609..03d1a6432 100644
--- a/templates/show_rules.inc.php
+++ b/templates/show_rules.inc.php
@@ -25,11 +25,11 @@ if ($playlist) {
 } else {
     $logic_operator = Core::get_request('operator');
 }
 $logic_operator = strtolower($logic_operator); ?>
 <script src="<?php echo AmpConfig::get('web_path'); ?>/lib/javascript/search.js"></script>
-<script src="<?php echo AmpConfig::get('web_path'); ?>/lib/javascript/search-data.php?type=<?php echo (string) filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES) ? scrub_out((string) filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES)) : 'song'; ?>"></script>
+<script src="<?php echo AmpConfig::get('web_path'); ?>/lib/javascript/search-data.php?type=<?php echo (string) scrub_out(filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES)) ?: 'song'; ?>"></script>
 
 <?php UI::show_box_top(T_('Rules') . "...", 'box box_rules'); ?>
 <table class="tabledata">
 <tbody id="searchtable">
     <tr id="rules_operator">
diff --git a/templates/show_search_form.inc.php b/templates/show_search_form.inc.php
index ee7ff111e..a8dea572f 100644
--- a/templates/show_search_form.inc.php
+++ b/templates/show_search_form.inc.php
@@ -19,11 +19,11 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  *
  */
 
 UI::show_box_top(T_('Search Ampache') . "...", 'box box_advanced_search'); ?>
-<form id="search" name="search" method="post" action="<?php echo AmpConfig::get('web_path'); ?>/search.php?type=<?php echo (string) filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES) ? scrub_out((string) filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES)) : 'song'; ?>" enctype="multipart/form-data" style="Display:inline">
+<form id="search" name="search" method="post" action="<?php echo AmpConfig::get('web_path'); ?>/search.php?type=<?php echo (string) scrub_out(filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES)) ?: 'song'; ?>" enctype="multipart/form-data" style="Display:inline">
 <table class="tabledata">
     <tr id="search_location">
     <td><?php if ((string) filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES) !== 'song') { ?>
         <a href="<?php echo AmpConfig::get('web_path'); ?>/search.php?type=song"><?php echo T_('Songs'); ?></a><?php
     } else {

Vulnerable code:                 // combine these as they all do the same thing just different tables
                $looking      = str_replace('myplayed', '', $rule[0]);
                $column       = ($looking == '') ? 'id' : $looking;
                $my_type      = ($looking == '') ? 'song' : $looking;
                $operator_sql = ((int) $sql_match_operator == 0) ? 'IS NULL' : 'IS NOT NULL';
                // played once per user
                $table['myplayed'] .= (!strpos((string) $table['myplayed'], "myplayed_" . $my_type . "_" . $userid)) ?
                    "LEFT JOIN (SELECT `object_id`, `object_type`, `user` FROM `object_count` " .
                    "WHERE `object_count`.`object_type` = '$my_type' AND `object_count`.`count_type` = 'stream' " .
                    "AND `object_count`.`user`=$userid GROUP BY `object_id`, `object_type`, `user`) AS " .
                    "`myplayed_" . $my_type . "_" . $userid . "` " .
                    "ON `song`.`$column`=`myplayed_" . $my_type . "_" . $userid . "`.`object_id` " .
                    "AND `myplayed_" . $my_type . "_" . $userid . "`.`object_type` = '$my_type' " : ' ';
                $where[] = "`myplayed_" . $my_type . "_" . $userid . "`.`object_id` $operator_sql";
        $object_ids = Random::advanced($_REQUEST['type'], $_POST);
<form id="random" method="post" enctype="multipart/form-data" action="<?php echo AmpConfig::get('web_path'); ?>/random.php?action=get_advanced&type=<?php echo Core::get_request('type') ? scrub_out(Core::get_request('type')) : 'song'; ?>">
<script src="<?php echo AmpConfig::get('web_path'); ?>/lib/javascript/search-data.php?type=<?php echo (string) filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES) ? scrub_out((string) filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES)) : 'song'; ?>"></script>
<form id="search" name="search" method="post" action="<?php echo AmpConfig::get('web_path'); ?>/search.php?type=<?php echo (string) filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES) ? scrub_out((string) filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES)) : 'song'; ?>" enctype="multipart/form-data" style="Display:inline">
Fixed code:                     // combine these as they all do the same thing just different tables
                    $looking      = str_replace('myplayed', '', $rule[0]);
                    $column       = ($looking == '') ? 'id' : $looking;
                    $my_type      = ($looking == '') ? 'song' : $looking;
                    $operator_sql = ((int) $sql_match_operator == 0) ? 'IS NULL' : 'IS NOT NULL';
                    // played once per user
                    $table['myplayed'] .= (!strpos((string) $table['myplayed'], "myplayed_" . $my_type . "_" . $userid)) ?
                        "LEFT JOIN (SELECT `object_id`, `object_type`, `user` FROM `object_count` " .
                        "WHERE `object_count`.`object_type` = '$my_type' AND `object_count`.`count_type` = 'stream' " .
                        "AND `object_count`.`user`=$userid GROUP BY `object_id`, `object_type`, `user`) AS " .
                        "`myplayed_" . $my_type . "_" . $userid . "` " .
                        "ON `song`.`$column`=`myplayed_" . $my_type . "_" . $userid . "`.`object_id` " .
                        "AND `myplayed_" . $my_type . "_" . $userid . "`.`object_type` = '$my_type' " : ' ';
                    $where[] = "`myplayed_" . $my_type . "_" . $userid . "`.`object_id` $operator_sql";
        $object_ids = Random::advanced(filter_input(INPUT_GET, 'type', FILTER_SANITIZE_SPECIAL_CHARS), $_POST);
<form id="random" method="post" enctype="multipart/form-data" action="<?php echo AmpConfig::get('web_path'); ?>/random.php?action=get_advanced&type=<?php echo (string) scrub_out(filter_input(INPUT_GET, 'type', FILTER_SANITIZE_SPECIAL_CHARS)) ?: 'song'; ?>">
<script src="<?php echo AmpConfig::get('web_path'); ?>/lib/javascript/search-data.php?type=<?php echo (string) scrub_out(filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES)) ?: 'song'; ?>"></script>
<form id="search" name="search" method="post" action="<?php echo AmpConfig::get('web_path'); ?>/search.php?type=<?php echo (string) scrub_out(filter_input(INPUT_GET, 'type', FILTER_SANITIZE_STRING, FILTER_FLAG_NO_ENCODE_QUOTES)) ?: 'song'; ?>" enctype="multipart/form-data" style="Display:inline">