SQL injection in SemanticDrilldown
Closed, ResolvedPublicSecurity
Actions

Assigned To

Authored By

	Seb35
	Apr 19 2022, 7:08 PM

Description

In SemanticDrilldown current master 044614d there is a SQL injection through:

the GET parameter "_cat" in the special page Special:BrowseData
read in class SDBrowseData here, passed to the object SDBrowseDataPage here
saved in property category in object SDBrowseDataPage here
transmitted in SDBrowseData::printAppliedFilterLine() to SDAppliedFilter::getAllOrValues() here
injected in the SQL request in SDAppliedFilter::getAllOrValues() here

The method DBrowseData::printAppliedFilterLine() is used to display the SMW values when a SMW property value is selected.

An example of URL is /Special:BrowseData?SomeProperty=SomeValue&_cat=SomeCategory'_OR_''='. The category SomeCategory'_OR_''=' has to exist on the wiki and the result is that the WHERE is bypassed and more values than requested are displayed.

Possibly more harmful SQL queries could be created, but it is limited by existing escaping '_' → ' ' (preventing use of all columns and tables containing an underscore) and the two remaining lines in the original SQL query (preventing easy use of SQL comments “--” since these two lines must be integrated in some SQL request to avoid a global SQL error).

I found this SQLi with phan-taint-check-plugin.

Affiliation: Wiki Valley

Details

Risk Rating: Low
Author Affiliation: Wikimedia Communities

Project	Branch	Lines +/-	Subject
mediawiki/extensions/SemanticDrilldown	REL1_35	+2 -1	Improve quoting in DB query
mediawiki/extensions/SemanticDrilldown	REL1_36	+2 -1	Improve quoting in DB query
mediawiki/extensions/SemanticDrilldown	REL1_37	+4 -3	Improve quoting in DB query
mediawiki/extensions/SemanticDrilldown	REL1_38	+4 -3	Improve quoting in DB query
mediawiki/extensions/SemanticDrilldown	master	+4 -3	Improve quoting in DB query

Customize query in gerrit

Related Objects

Mentioned Here: rESDD044614d60c30: Localisation updates from https://translatewiki.net.

Event Timeline

Seb35 created this task.Apr 19 2022, 7:08 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 19 2022, 7:08 PM

Seb35 added a project: MediaWiki-extensions-SemanticDrilldown.Apr 19 2022, 7:08 PM

Here is a patch as a Git commit:

commit dda8ac5ca2d8f6ff595f1a2e641bda4c2057bfc4
tree 7492d30ceab679b68d72c026ecbddc7987085202
parent 044614d60c30cbbd7d3882f836905796bc1ade06
author Sébastien Beyou <sebastien.beyou@wiki-valley.com> 1650395410 +0200
committer Sébastien Beyou <sebastien.beyou@wiki-valley.com> 1650395410 +0200

    SECURITY: SQL injection
    
    Through the URL GET parameter _cat. The exploitation is limited by:
    * the fact it must be a category title: in particular _ are replaced by
      spaces, preventing use of most SQL tables and columns having a _;
    * the category title must exist, hence limited to editors;
    * there are next lines in the SQL request, preventing comment-type SQLi.
    
    Bug: T306463
    Change-Id: Ia4d588b91e71d41e594e23684b1ec24ba1f1db0c

diff --git includes/SDAppliedFilter.php includes/SDAppliedFilter.php
index dc7c7c5..689fdc6 100644
--- includes/SDAppliedFilter.php
+++ includes/SDAppliedFilter.php
@@ -197,6 +197,7 @@ class SDAppliedFilter {
 		$property_value = $this->filter->escaped_property;
 		$dbr = wfGetDB( DB_REPLICA );
 		$property_table_name = $dbr->tableName( $this->filter->getTableName() );
+		$category = $dbr->addQuotes( $category );
 		if ( $this->filter->property_type != 'date' ) {
 			$value_field = $this->filter->getValueField();
 		} else {
@@ -226,7 +227,7 @@ class SDAppliedFilter {
 	JOIN $smwIDs cat_ids ON insts.o_id = cat_ids.smw_id
 	WHERE p_ids.smw_title = '$property_value'
 	AND cat_ids.smw_namespace = $cat_ns
-	AND cat_ids.smw_title = '$category'
+	AND cat_ids.smw_title = $category
 	GROUP BY $value_field
 	ORDER BY $value_field";
 		$res = $dbr->query( $sql );