Fix CVE-2014-1609: SQL injection vulnerabilities

Additional cases of db_query() instead of db_query_bound() usage, potentially allowing SQL injection attacks due to unsanitized use of parameters within the query. This includes vboctor's 2 comments. Fixes #16880 Signed-off-by: Damien Regad <dregad@mantisbt.org>
mantisbt · Jan 24, 2014 · 7d76827 · 7d76827
1 parent 3be86ce
commit 7d76827
Show file tree

Hide file tree

Showing 8 changed files with 52 additions and 42 deletions.
diff --git a/admin/db_stats.php b/admin/db_stats.php
@@ -51,7 +51,7 @@ function print_info_row( $p_description, $p_value ) {
 function helper_table_row_count( $p_table ) {
 	$t_table = $p_table;
 	$t_query = "SELECT COUNT(*) FROM $t_table";
-	$t_result = db_query( $t_query );
+	$t_result = db_query_bound( $t_query );
 	$t_count = db_result( $t_result );
 
 	return $t_count;

diff --git a/api/soap/mc_project_api.php b/api/soap/mc_project_api.php
@@ -715,14 +715,14 @@ function mc_project_get_attachments( $p_username, $p_password, $p_project_id ) {
 		FROM $t_project_file_table pft
 		LEFT JOIN $t_project_table pt ON pft.project_id = pt.id
 		LEFT JOIN $t_project_user_list_table pult
-		ON pft.project_id = pult.project_id AND pult.user_id = $t_user_id
-		LEFT JOIN $t_user_table ut ON ut.id = $t_user_id
+		ON pft.project_id = pult.project_id AND pult.user_id = " . db_param() . "
+		LEFT JOIN $t_user_table ut ON ut.id = " . db_param() . "
 		WHERE pft.project_id in (" . implode( ',', $t_projects ) . ") AND
-		( ( ( pt.view_state = $t_pub OR pt.view_state is null ) AND pult.user_id is null AND ut.access_level $t_access_clause ) OR
-		( ( pult.user_id = $t_user_id ) AND ( pult.access_level $t_access_clause ) ) OR
-		( ut.access_level = $t_admin ) )
+		( ( ( pt.view_state = " . db_param() . " OR pt.view_state is null ) AND pult.user_id is null AND ut.access_level $t_access_clause ) OR
+		( ( pult.user_id = " . db_param() . " ) AND ( pult.access_level $t_access_clause ) ) OR
+		( ut.access_level = " . db_param() . " ) )
 		ORDER BY pt.name ASC, pft.title ASC";
-	$result = db_query( $query );
+	$result = db_query_bound( $query, array( $t_user_id, $t_user_id, $t_pub, $t_user_id, $t_admin ) );
 	$num_files = db_num_rows( $result );
 
 	$t_result = array();

diff --git a/core/news_api.php b/core/news_api.php
@@ -295,13 +295,15 @@ function news_get_limited_rows( $p_offset, $p_project_id = null ) {
 
 			if( 1 == count( $t_projects ) ) {
 				$c_project_id = $t_projects[0];
-				$query .= " WHERE project_id='$c_project_id'";
+				$query .= " WHERE project_id=" . db_params();
+				$t_params = array( $c_project_id );
 			} else {
 				$query .= ' WHERE project_id IN (' . join( $t_projects, ',' ) . ')';
+				$t_params = null;
 			}
 
 			$query .= ' ORDER BY announcement DESC, id DESC';
-			$result = db_query( $query, $t_news_view_limit, $c_offset );
+			$result = db_query_bound( $query, $t_params, $t_news_view_limit, $c_offset );
 			break;
 		case 1:
 

diff --git a/core/summary_api.php b/core/summary_api.php
@@ -114,7 +114,7 @@ function summary_print_by_enum( $p_enum ) {
 				WHERE $t_project_filter
 				GROUP BY $p_enum $t_status_query
 				ORDER BY $p_enum $t_status_query";
-	$result = db_query( $query );
+	$result = db_query_bound( $query );
 
 	$t_last_value = -1;
 	$t_bugs_open = 0;
@@ -423,10 +423,10 @@ function summary_print_by_age() {
 		return;
 	}
 	$query = "SELECT * FROM $t_mantis_bug_table
-				WHERE status < $t_resolved
+				WHERE status < " . db_param() . "
 				AND $specific_where
 				ORDER BY date_submitted ASC, priority DESC";
-	$result = db_query( $query );
+	$result = db_query_bound( $query, array( $t_resolved ) );
 
 	$t_count = 0;
 	$t_private_bug_threshold = config_get( 'private_bug_threshold' );
@@ -474,7 +474,7 @@ function summary_print_by_developer() {
 				WHERE handler_id>0 AND $specific_where
 				GROUP BY handler_id, status
 				ORDER BY handler_id, status";
-	$result = db_query( $query );
+	$result = db_query_bound( $query );
 
 	$t_last_handler = -1;
 	$t_bugs_open = 0;
@@ -577,7 +577,7 @@ function summary_print_by_reporter() {
 				WHERE $specific_where
 				GROUP BY reporter_id
 				ORDER BY num DESC";
-	$result = db_query( $query, $t_reporter_summary_limit );
+	$result = db_query_bound( $query, null, $t_reporter_summary_limit );
 
 	$t_reporters = array();
 	while( $row = db_fetch_array( $result ) ) {
@@ -589,11 +589,11 @@ function summary_print_by_reporter() {
 	foreach( $t_reporters as $t_reporter ) {
 		$v_reporter_id = $t_reporter;
 		$query = "SELECT COUNT(id) as bugcount, status FROM $t_mantis_bug_table
-					WHERE reporter_id=$v_reporter_id
+					WHERE reporter_id=" . db_param() . "
 					AND $specific_where
 					GROUP BY status
 					ORDER BY status";
-		$result2 = db_query( $query );
+		$result2 = db_query_bound( $query, array( $v_reporter_id ) );
 
 		$last_reporter = -1;
 		$t_bugs_open = 0;
@@ -663,7 +663,7 @@ function summary_print_by_category() {
 				GROUP BY $t_project_query c.name, b.category_id, b.status
 				ORDER BY $t_project_query c.name";
 
-	$result = db_query( $query );
+	$result = db_query_bound( $query );
 
 	$last_category_name = -1;
 	$last_category_id = -1;

diff --git a/plugins/MantisGraph/core/graph_api.php b/plugins/MantisGraph/core/graph_api.php
@@ -627,11 +627,15 @@ function create_bug_enum_summary( $p_enum_string, $p_enum ) {
 	$t_metrics = array();
 	$t_assoc_array = MantisEnum::getAssocArrayIndexedByValues( $p_enum_string );
 
+	if( !db_field_exists( $p_enum, $t_bug_table ) ) {
+		trigger_error( ERROR_DB_FIELD_NOT_FOUND, ERROR );
+	}
+
 	foreach ( $t_assoc_array as $t_value => $t_label  ) {
 		$query = "SELECT COUNT(*)
 					FROM $t_bug_table
-					WHERE $p_enum='$t_value' $specific_where";
-		$result = db_query( $query );
+					WHERE $p_enum=" . db_param() . " $specific_where";
+		$result = db_query_bound( $query, array( $t_value ) );
 		$t_metrics[$t_label] = db_result( $result, 0 );
 	}
 
@@ -655,32 +659,36 @@ function enum_bug_group( $p_enum_string, $p_enum ) {
 	$t_clo_val = config_get( 'bug_closed_status_threshold' );
 	$specific_where = " AND " . helper_project_specific_where( $t_project_id, $t_user_id );
 
+	if( !db_field_exists( $p_enum, $t_bug_table ) ) {
+		trigger_error( ERROR_DB_FIELD_NOT_FOUND, ERROR );
+	}
+
 	$t_array_indexed_by_enum_values = MantisEnum::getAssocArrayIndexedByValues( $p_enum_string );
 	$enum_count = count( $t_array_indexed_by_enum_values );
 	foreach ( $t_array_indexed_by_enum_values as $t_value => $t_label ) {
 		# Calculates the number of bugs opened and puts the results in a table
 		$query = "SELECT COUNT(*)
 					FROM $t_bug_table
-					WHERE $p_enum='$t_value' AND
-						status<'$t_res_val' $specific_where";
-		$result2 = db_query( $query );
+					WHERE $p_enum=" . db_param() . " AND
+						status<" . db_param() . " $specific_where";
+		$result2 = db_query( $query, array( $t_value, $t_res_val ) );
 		$t_metrics['open'][$t_label] = db_result( $result2, 0, 0 );
 
 		# Calculates the number of bugs closed and puts the results in a table
 		$query = "SELECT COUNT(*)
 					FROM $t_bug_table
-					WHERE $p_enum='$t_value' AND
-						status>='$t_clo_val' $specific_where";
-		$result2 = db_query( $query );
+					WHERE $p_enum=" . db_param() . " AND
+						status>=" . db_param() . " $specific_where";
+		$result2 = db_query_bound( $query, array( $t_value, $t_clo_val ) );
 		$t_metrics['closed'][$t_label] = db_result( $result2, 0, 0 );
 
 		# Calculates the number of bugs resolved and puts the results in a table
 		$query = "SELECT COUNT(*)
 					FROM $t_bug_table
-					WHERE $p_enum='$t_value' AND
-						status>='$t_res_val'  AND
-						status<'$t_clo_val' $specific_where";
-		$result2 = db_query( $query );
+					WHERE $p_enum=" . db_param() . " AND
+						status>=" . db_param() . " AND
+						status<" . db_param() . " $specific_where";
+		$result2 = db_query_bound( $query, array(  $t_value, $t_res_val, $t_clo_val ) );
 		$t_metrics['resolved'][$t_label] = db_result( $result2, 0, 0 );
 	}
 
@@ -875,12 +883,12 @@ function create_cumulative_bydate() {
 			FROM $t_bug_table LEFT JOIN $t_history_table
 			ON $t_bug_table.id = $t_history_table.bug_id
 			WHERE $specific_where
-						AND $t_bug_table.status >= '$t_res_val'
-						AND ( ( $t_history_table.new_value >= '$t_res_val'
+						AND $t_bug_table.status >= " . db_param() . "
+						AND ( ( $t_history_table.new_value >= " . db_param() . "
 								AND $t_history_table.field_name = 'status' )
 						OR $t_history_table.id is NULL )
 			ORDER BY $t_bug_table.id, date_modified ASC";
-	$result = db_query( $query );
+	$result = db_query( $query, array( $t_res_val, $t_res_val ) );
 	$bug_count = db_num_rows( $result );
 
 	$t_last_id = 0;

diff --git a/plugins/MantisGraph/pages/bug_graph_bycategory.php b/plugins/MantisGraph/pages/bug_graph_bycategory.php
@@ -105,9 +105,9 @@
 	' WHERE bug_id in ('.implode(',', $t_bug).') and '.
 		'( (type='.NORMAL_TYPE.' and field_name=\'category\') or '.
 			'(type='.NORMAL_TYPE.' and field_name=\'status\') or type='.NEW_BUG.' ) and '.
-			'date_modified >= \''. $t_start .'\''.
+			'date_modified >= ' . db_param() .
 		' order by date_modified DESC';
-$t_result = db_query( $t_select );
+$t_result = db_query_bound( $t_select, array( $t_start ) );
 $row = db_fetch_array( $t_result );
 
 for ($t_now = time() - $t_incr; $t_now >= $t_start; $t_now -= $t_incr) {

diff --git a/plugins/MantisGraph/pages/bug_graph_bystatus.php b/plugins/MantisGraph/pages/bug_graph_bystatus.php
@@ -100,9 +100,9 @@
 $t_select = 'SELECT bug_id, type, old_value, new_value, date_modified FROM '.$t_bug_hist_table.
 	' WHERE bug_id in ('.implode(',', $t_bug).
 	') and ( (type='.NORMAL_TYPE.' and field_name=\'status\')
-		or type='.NEW_BUG.' ) and date_modified >= \''. $t_start .'\''.
+		or type='.NEW_BUG.' ) and date_modified >= ' . db_param() .
 	' order by date_modified DESC';
-$t_result = db_query( $t_select );
+$t_result = db_query_bound( $t_select, array( $t_start ) );
 $t_row = db_fetch_array( $t_result );
 
 for ($t_now = time() - $t_incr; $t_now >= $t_start; $t_now -= $t_incr) {

diff --git a/proj_doc_page.php b/proj_doc_page.php
@@ -99,14 +99,14 @@
 			FROM $t_project_file_table pft
 				LEFT JOIN $t_project_table pt ON pft.project_id = pt.id
 				LEFT JOIN $t_project_user_list_table pult
-					ON pft.project_id = pult.project_id AND pult.user_id = $t_user_id
-				LEFT JOIN $t_user_table ut ON ut.id = $t_user_id
+					ON pft.project_id = pult.project_id AND pult.user_id = " . db_param() . "
+				LEFT JOIN $t_user_table ut ON ut.id = " . db_param() . "
 			WHERE pft.project_id in (" . implode( ',', $t_projects ) . ") AND
-				( ( ( pt.view_state = $t_pub OR pt.view_state is null ) AND pult.user_id is null AND ut.access_level $t_access_clause ) OR
-					( ( pult.user_id = $t_user_id ) AND ( pult.access_level $t_access_clause ) ) OR
-					( ut.access_level >= $t_admin ) )
+				( ( ( pt.view_state = " . db_param() . " OR pt.view_state is null ) AND pult.user_id is null AND ut.access_level $t_access_clause ) OR
+					( ( pult.user_id = " . db_param() . " ) AND ( pult.access_level $t_access_clause ) ) OR
+					( ut.access_level >= " . db_param() . " ) )
 			ORDER BY pt.name ASC, pft.title ASC";
-$result = db_query( $query );
+$result = db_query_bound( $query, array( $t_user_id, $t_user_id, $t_pub, $t_user_id, $t_admin ) );
 
 html_page_top( lang_get( 'docs_link' ) );
 ?>