View Issue Details

WikiRelated ChangesetsJump to Notes
IDProjectCategoryView StatusDate SubmittedLast Update
0015573mantisbtsecuritypublic2013-03-06 10:322014-09-23 18:05
Reporterjjtest Assigned Todregad  
PriorityhighSeveritycrashReproducibilityalways
Status closedResolutionfixed 
PlatformUnixOSSolarisOS Version8/10
Product Version1.2.12 
Target Version1.2.15Fixed in Version1.2.15 
Summary0015573: CVE-2013-1883: One query can be issued via current Mantis interface to take down site
Description

This is a very bad query problem that can be used as a DOS attack very easily. I found it while upgrading 1.1.18 to 1.2.14. When I ran it on your site yesterday... somewhere around 5pm EST, it took you down too! (Sorry but proved the issue!)

In my situation, I had to kill the offending thread in MySQL and restart Apache to get my site back online.

The query caused MySQL to take all of the cpu on my solaris box...

It has to do with the search feature "Match Type: Any Condition"

Basically it is broken badly WHEN USED WITH A TEXT STRING SEARCH.

All you have to do to reproduce this is search a large dataset for any text string from the View Issues search box.

Searching with the default "Match Type: All Conditions" should yield expected results.

Now switch Match Type to "Any Condition" and press Apply Filter.... and watch your site... die.

This is all compounded (TERRIBLY) by the feature that stores your last View Issues query and automatically reissues it when you click View Issues.... now you DOS your site every time you click View Issues!

To get control back from this mishap I was forced to remove my cookie so the offending search was not automatically reissued.

Steps To Reproduce

All you have to do to reproduce this is search a large dataset for any text string from the View Issues search box.

Searching with the default "Match Type: All Conditions" should yield expected results.

Now switch Match Type to "Any Condition" and press Apply Filter.... and watch your site... die.

Additional Information

When using the fast All Conditions to search for the string "sql", this query is issued:

Count( DISTINCT mantis_bug_table.id ) as idcnt FROM mantis_bug_text_table, man
tis_project_table, mantis_bug_table LEFT JOIN mantis_bugnote_table ON mantis_bu
g_table.id = mantis_bugnote_table.bug_id LEFT JOIN mantis_bugnote_text_table ON
mantis_bugnote_table.bugnote_text_id = mantis_bugnote_text_table.id WHERE manti
s_project_table.enabled = 1 AND mantis_project_table.id = mantis_bug_table.proje
ct_id AND ( mantis_bug_table.project_id in (6, 282, 94, 151, 176, 376, 194, 321,
199, 44, 43, 111, 273, 45, 41, 58, 155, 333, 48, 297, 298, 213, 299) ) AND ( ma
ntis_bug_table.bug_text_id = mantis_bug_text_table.id AND ( ( (summary LIKE '%sq
l%') OR (mantis_bug_text_table.description LIKE '%sql%') OR (mantis_bug_text_tab
le.steps_to_reproduce LIKE '%sql%') OR (mantis_bug_text_table.additional_informa
tion LIKE '%sql%') OR (mantis_bugnote_text_table.note LIKE '%sql%') ) ) )

When using the tragically slow "Any Condition" query for the string "sql", this query is issued:

Count( DISTINCT mantis_bug_table.id ) as idcnt FROM mantis_bug_text_table, man
tis_project_table, mantis_bug_table LEFT JOIN mantis_bugnote_table ON mantis_bu
g_table.id = mantis_bugnote_table.bug_id LEFT JOIN mantis_bugnote_text_table ON
mantis_bugnote_table.bugnote_text_id = mantis_bugnote_text_table.id WHERE manti
s_project_table.enabled = 1 AND mantis_project_table.id = mantis_bug_table.proje
ct_id AND ( mantis_bug_table.project_id in (6, 282, 94, 151, 176, 376, 194, 321,
199, 44, 43, 111, 273, 45, 41, 58, 155, 333, 48, 297, 298, 213, 299) ) AND ( ma
ntis_bug_table.bug_text_id = mantis_bug_text_table.id OR ( ( (summary LIKE '%sql
%') OR (mantis_bug_text_table.description LIKE '%sql%') OR (mantis_bug_text_tabl
e.steps_to_reproduce LIKE '%sql%') OR (mantis_bug_text_table.additional_informat
ion LIKE '%sql%') OR (mantis_bugnote_text_table.note LIKE '%sql%') ) ) )

Basically, you cannot "OR" the primary key of the LEFT-JOINED mantis_bug_text table, while attempting to match against it's description field. Well...you obviously CAN...but the results are tragic.

When I change that OR to an AND the query runs quickly.

TagsNo tags attached.
Attached Files
15573-fix-1.diff (728 bytes)    
diff --git a/core/filter_api.php b/core/filter_api.php
index 55edd46..7b964b8 100644
--- a/core/filter_api.php
+++ b/core/filter_api.php
@@ -1996,8 +1996,7 @@
 		# add text query elements to arrays
 		if ( !$t_first ) {
 			$t_from_clauses[] = "$t_bug_text_table";
-			$t_where_clauses[] = "$t_bug_table.bug_text_id = $t_bug_text_table.id";
-			$t_where_clauses[] = $t_textsearch_where_clause;
+			$t_where_clauses[] = "$t_bug_table.bug_text_id = $t_bug_text_table.id AND $t_textsearch_where_clause";
 			$t_join_clauses[] = " LEFT JOIN $t_bugnote_table ON $t_bug_table.id = $t_bugnote_table.bug_id";
 			$t_join_clauses[] = " LEFT JOIN $t_bugnote_text_table ON $t_bugnote_table.bugnote_text_id = $t_bugnote_text_table.id";
 		}
15573-fix-1.diff (728 bytes)   

Relationships

Relationship GraphDependency Graph
related to 0006809 closedrombert Using an 'Or' filter logic 
related to 0015678 closeddregad Bad performance when filtering and using match type "Any Condition" 
related to 0015721 closedgrangeway Functionality to consider porting to master-2.0.x 
related to 0016922 closeddregad View Issues list is very slow if a filter is used 

Activities

jjtest

jjtest

2013-03-06 10:38

reporter   ~0035394

Basically..I think you may only have one "OR" in the water!

Maybe you meant the OR to replace the AND between:

...213, 299) ) AND ( mantis_bug_table.bug_text_id...

instead of the current:

...mantis_bug_table.bug_text_id = mantis_bug_text_table.id OR ( ( (summary LIKE...
jjtest

jjtest

2013-03-06 10:40

reporter   ~0035395

I was upgrading from 1.1.8... not 1.1.18
dregad

dregad

2013-03-06 19:36

developer   ~0035400

Thanks for the bug report. I'm marking this issue as private for now, to avoid giving ideas to some people...
jjtest

jjtest

2013-03-07 10:31

reporter   ~0035408

I know. I just hate those "some people" too... they have sucked a lot of the fun out of my job in the last decade or so...
rombert

rombert

2013-03-07 16:04

reporter   ~0035409

Thanks for the report and the detailed analysis, it has been quite helpful in narrowing down the root cause.

I have attached a fix which works for me ™ but I'd be happy if you could validate that it fixes the performance problem and also returns the correct results.
dregad

dregad

2013-03-08 06:11

developer   ~0035414

I tested the proposed patch on my dev box, using an old clone of our tracker DB (about 11'000 bugs and 30'000 notes). The query's performance with ANY condition is about the same as for ALL (whereas before I killed mysql after 30 minutes of execution).

As a side note and while on the topic of performance, I'm not sure why we are using an outer join on the bugnote_text table, in theory it should be a 1:1 relationship so an inner join would be enough and be more efficient.

It may also be useful from a readability perspective, to rewrite the query to use a JOIN clause for the mantis_bug_text_table and mantis_project_table. This way the where clauses would only relate to the filter criteria.
rombert

rombert

2013-03-08 06:37

reporter   ~0035416

As a side note and while on the topic of performance, I'm not sure why we are using an outer join on the bugnote_text table, in theory it should be a 1:1 relationship so an inner join would be enough and be more efficient.

Wouldn't an inner join require that the bugnote_text_table exist?
dregad

dregad

2013-03-08 12:14

developer   ~0035422

Yes it would, but an outer join does also - not sure I get your point...
rombert

rombert

2013-03-08 14:28

reporter   ~0035424

Um, sorry. I'm less fluent in SQL by the day :-) I meant that an inner join would require a bugnote to exist, whereas the outer join would allow matching even if no matching bugnote entry exists.
dregad

dregad

2013-03-11 05:04

developer   ~0035427

You're absolutely right on principle.

However in this case as I said before bugnote_text and bugnote tables have a 1:1 relationship, i.e. there is (or should be) always one record in each table for a given note, except possibly in case of errors (in which case we'd likely have orphan records in bugnote_text table). See bugnote_add() lines 154 & 176.

It's worth noting that the same logic also applies to bug & bug_text tables.
rombert

rombert

2013-03-11 06:05

reporter   ~0035428

Oh right, I misread that as bug_table and bugnote_table.
rombert

rombert

2013-03-12 17:18

reporter   ~0035443

I've actually left this unchanged since I don't want to muck around in the filter_api anymore ... sigh. Hopefully there will come a time for cleanups as well.
atrol

atrol

2013-03-12 18:09

developer   ~0035444

dregad wrote at 0015573:0035414

I tested the proposed patch on my dev box, using an old clone of our tracker DB

I did the same, using latest code from github.
I still get the issue.
rombert

rombert

2013-03-13 03:39

reporter   ~0035447

Reopening based on atrol's indication that the fix does not work.
dregad

dregad

2013-03-13 03:58

developer   ~0035448

I did the same, using latest code from github. I still get the issue.

Strange. I'll re-test and let you know (likely not before next week though)
dregad

dregad

2013-03-18 14:49

developer   ~0035905

Not sure how I tested this the last time... Now I can consistently reproduce the problem.

I think the problem is caused by mixing join criteria with where clauses. I'll dump the SQL statements and have a closer look later tonight.
dregad

dregad

2013-03-18 19:26

developer   ~0035906

That should (hopefully) do the trick. Please test and let me know.

Note: I also pushed one additional commit implementing my suggestion in 0015573:0035414.
atrol

atrol

2013-03-18 19:34

developer   ~0035907

Made some quick tests, looks good at first sight.
vboctor

vboctor

2013-03-20 15:42

manager   ~0035937

I would love to see bug and bug_text tables merged into one at one point. Not sure if it is worth the work, but I always thought of this as extra complexity.

However, I do agree with atrol's recommendation over email that we should deploy this to the official / demo bug trackers and once validated for few days kick-off 1.2.15.
dregad

dregad

2013-03-21 05:13

developer   ~0035943

I would love to see bug and bug_text tables merged into one at one point.
Not sure if it is worth the work, but I always thought of this as extra complexity.

Agreed. And the same goes for bugnote vs bugnote_text too. But this is no small undertaking, and definitely not on the critical path atm... Not to mention that it obviously involves a schema change, which means it has to be done in master branch, and we still have no plan or resources to get cracking on releasing that ;)

That said, I have updated both trackers with the fix now, so I'll make this issue public.
dregad

dregad

2013-03-21 06:32

developer   ~0035944

A CVE ID for this issue was requested from oss-security mailing list [1] on 21-Mar-2013.

[1] http://article.gmane.org/gmane.comp.security.oss.general/9779
dregad

dregad

2013-03-22 04:30

developer   ~0035947

CVE-2013-1883 was assigned on 22-Mar-2013
grangeway

grangeway

2013-04-05 17:56

reporter   ~0036068

Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch

Related Changesets

MantisBT: master-1.2.x 543ba012

2013-03-07 16:02

rombert


Details Diff		 filter_api: ensure that the free_text where clauses are always ANDed

Fixes 0015573: One query can be issued via current Mantis interface to
take down site		 Affected Issues
0015573
mod - core/filter_api.php Diff File

MantisBT: master 540ae471

2013-03-07 16:02

rombert


Details Diff		 filter_api: ensure that the free_text where clauses are always ANDed

Fixes 0015573: One query can be issued via current Mantis interface to
take down site		 Affected Issues
0015573
mod - core/filter_api.php Diff File

MantisBT: master ce961095

2013-03-18 18:26

dregad


Details Diff		 Revert "filter_api: ensure that the free_text where clauses are always ANDed"

This reverts commit 543ba012999c20310a2208bd8e5f7f88efe392ce.		 Affected Issues
0015573
mod - core/filter_api.php Diff File

MantisBT: master-1.2.x d4e7b224

2013-03-18 18:26

dregad


Details Diff		 Revert "filter_api: ensure that the free_text where clauses are always ANDed"

This reverts commit 543ba012999c20310a2208bd8e5f7f88efe392ce.		 Affected Issues
0015573
mod - core/filter_api.php Diff File

MantisBT: master 44055f88

2013-03-18 18:36

dregad


Details Diff		 Fix filter api issue with 'any condition' and text search

A filter combining some criteria and a text search with 'any condition'
results in a cartesian product, which has the potential to bring down
the site as the RDBMS eats up all available resources.

The root cause of this behavior is joining the bug_text table with a
from clause and setting the join's criteria in the query's where clause,
without taking consideration the operator's precedence (AND/OR).

This commit resolves the problem by using a JOIN clause instead, which
makes the query cleaner.

Fixes 0015573		 Affected Issues
0015573, 0016922
mod - core/filter_api.php Diff File

MantisBT: master-1.2.x d16988c3

2013-03-18 18:36

dregad


Details Diff		 Fix filter api issue with 'any condition' and text search

A filter combining some criteria and a text search with 'any condition'
results in a cartesian product, which has the potential to bring down
the site as the RDBMS eats up all available resources.

The root cause of this behavior is joining the bug_text table with a
from clause and setting the join's criteria in the query's where clause,
without taking consideration the operator's precedence (AND/OR).

This commit resolves the problem by using a JOIN clause instead, which
makes the query cleaner.

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