View Issue Details

IDProjectCategoryView StatusLast Update
0017841mantisbtsecuritypublic2014-12-22 08:22
Reporteredwingozeling Assigned Tovboctor  
PriorityhighSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version1.2.17 
Target Version1.2.18Fixed in Version1.2.18 
Summary0017841: CVE-2014-9089: SQL injection in view_all_set.php
Description

Both the 'sort' and 'dir' parameters to view_all_set.php are validated insufficiently before they are used in queries by view_all_bug_page.php.

Both parameters are split into chunks on ','. After splitting, only the first two values are validated. By supplying a third value, SQL injection can be performed.

Steps To Reproduce

Browse to /view_all_set.php?sort=id,status,severity'&dir=ASC,ASC,ASC&type=2 on a Mantis BugTracker installation.

A redirect towards view_all_bug_page.php occurs, on which an SQL error will be displayed.
As the injection is placed within the ORDER BY clause, and the name of mantis_bug_table is placed before each chunk, conditional statements cannot be introduced. It is however possible to add INTO OUTFILE. Depending on the database driver, stacked queries may also be possible.

Additional Information

Please note the ' after mantis_bug_table.order in the following error message.

1064: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' DESC, mantis_bug_table.last_updated DESC, mantis_bug_table.date_submitted DESC' at line 1 voor de zoekopdracht: SELECT DISTINCT mantis_bug_table.* FROM mantis_bug_table JOIN mantis_project_table ON mantis_project_table.id = mantis_bug_table.project_id WHERE mantis_project_table.enabled = ? AND ( mantis_bug_table.project_id = 1 ) ORDER BY mantis_bug_table.id DESC, mantis_bug_table.severity DESC, mantis_bug_table.order' DESC, mantis_bug_table.last_updated DESC, mantis_bug_table.date_submitted DESC

TagsNo tags attached.
Attached Files
0001-Improve-validation-for-filter-sort-and-direction.patch (1,163 bytes)   
From a8fb502cbd69b2805405349f4ad21bde49bf1e5c Mon Sep 17 00:00:00 2001
From: Victor Boctor <victor@mantishub.net>
Date: Mon, 24 Nov 2014 20:54:51 -0800
Subject: [PATCH] Improve validation for filter sort and direction

Fixes #17841
---
 core/filter_api.php | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/core/filter_api.php b/core/filter_api.php
index 7ec33d7..0667996 100644
--- a/core/filter_api.php
+++ b/core/filter_api.php
@@ -606,8 +606,20 @@ function filter_ensure_valid_filter( $p_filter_arr ) {
 			unset( $t_fields[$i] );
 		}
 	}
+
+	# Make sure array is no longer than 2 elements
 	$t_sort_fields = explode( ',', $p_filter_arr['sort'] );
+	if( count( $t_sort_fields ) > 2 ) {
+		$t_sort_fields = array_slice( $t_sort_fields, 0, 2 );
+	}
+
+	# Make sure array is no longer than 2 elements
 	$t_dir_fields = explode( ',', $p_filter_arr['dir'] );
+	if( count( $t_dir_fields ) > 2 ) {
+		$t_dir_fields = array_slice( $t_dir_fields, 0, 2 );
+	}
+
+	# Validate the max of two segments for $t_sort_fields and $t_dir_fields
 	for( $i = 0;$i < 2;$i++ ) {
 		if( isset( $t_sort_fields[$i] ) ) {
 			$t_drop = false;
-- 
1.9.3 (Apple Git-50)

Activities

dregad

dregad

2014-11-06 09:44

developer   ~0041773

Thanks for the bug report. I'll investigate and get back to you.

If you have already reserved a CVE for this, please let us know the ID.

edwingozeling

edwingozeling

2014-11-06 10:16

reporter   ~0041774

You are welcome, I have not reserved a CVE for this.

A POC: access the following URL:
view_all_set.php?sort=id,severity,status+DESC+INTO+OUTFILE+%27/tmp/sql_dump.txt%27+--&dir=ASC,ASC,ASC&type=2

While you get an error message, the file '/tmp/sql_dump.txt' will still be created and contains the query results. (results may vary based on OS and privileges of the MySQL-account)

vboctor

vboctor

2014-11-24 23:58

manager   ~0041899

I've attached a patch [1] that does the validation as part of filter_ensure_valid_filter() API and hence it will handle new injections as well as already saved ones.

[1] 0001-Improve-validation-for-filter-sort-and-direction.patch

edwingozeling

edwingozeling

2014-11-25 03:13

reporter   ~0041900

Seems like a proper solution. One question, what is the added bennefit of the following line?

if( count( $t_sort_fields ) > 2 ) {

If I'm correct, just performing array_slice() would yield the same effect.

vboctor

vboctor

2014-11-25 10:16

manager   ~0041901

The array_slice() documentation doesn't indicate that it can slice larger than the array size. The code also doesn't always get an array of two or more from memory. So it is defensive programming.

edwingozeling

edwingozeling

2014-11-25 10:26

reporter   ~0041902

While I'm all for defensive programming, the documentation does specify it can handle shorter arrays:
From http://php.net/array_slice
"If the array is shorter than the length, then only the available array elements will be present."

Anyway, good job on the patch, I hope to see it released soon.

dregad

dregad

2014-11-25 18:16

developer   ~0041917

CVE request sent http://thread.gmane.org/gmane.comp.security.oss.general/14884

Related Changesets

MantisBT: master-1.2.x b0021673

2014-11-24 18:54

vboctor

Committer: dregad


Details Diff
Improve validation for filter sort and direction

Fixes 0017841
Affected Issues
0017841
mod - core/filter_api.php Diff File

MantisBT: master ac817e3f

2014-11-24 18:54

vboctor

Committer: dregad


Details Diff
Improve validation for filter sort and direction

Fixes 0017841
Affected Issues
0017841
mod - core/filter_api.php Diff File