Page MenuHomePhabricator

SQL injection in SemanticDrilldown
Closed, ResolvedPublicSecurity

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

Event Timeline

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 );
Yaron_Koren claimed this task.

Thanks for this patch. I added this in here, so I believe the problem is fixed now:

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SemanticDrilldown/+/785213

sbassett changed Author Affiliation from Other (Please specify in description) to Wikimedia Communities.
sbassett edited projects, added SecTeam-Processed; removed Security-Team.
sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".
sbassett changed the edit policy from "Custom Policy" to "All Users".
sbassett changed Risk Rating from N/A to Low.

Change 786425 had a related patch set uploaded (by RhinosF1; author: Yaron Koren):

[mediawiki/extensions/SemanticDrilldown@REL1_38] Improve quoting in DB query

https://gerrit.wikimedia.org/r/786425

Change 787086 had a related patch set uploaded (by RhinosF1; author: Yaron Koren):

[mediawiki/extensions/SemanticDrilldown@REL1_37] Improve quoting in DB query

https://gerrit.wikimedia.org/r/787086

Change 787087 had a related patch set uploaded (by RhinosF1; author: Yaron Koren):

[mediawiki/extensions/SemanticDrilldown@REL1_36] Improve quoting in DB query

https://gerrit.wikimedia.org/r/787087

Change 787088 had a related patch set uploaded (by RhinosF1; author: Yaron Koren):

[mediawiki/extensions/SemanticDrilldown@REL1_35] Improve quoting in DB query

https://gerrit.wikimedia.org/r/787088

Change 786425 merged by jenkins-bot:

[mediawiki/extensions/SemanticDrilldown@REL1_38] Improve quoting in DB query

https://gerrit.wikimedia.org/r/786425

Change 787086 merged by jenkins-bot:

[mediawiki/extensions/SemanticDrilldown@REL1_37] Improve quoting in DB query

https://gerrit.wikimedia.org/r/787086

Change 787087 merged by jenkins-bot:

[mediawiki/extensions/SemanticDrilldown@REL1_36] Improve quoting in DB query

https://gerrit.wikimedia.org/r/787087

Change 787088 merged by jenkins-bot:

[mediawiki/extensions/SemanticDrilldown@REL1_35] Improve quoting in DB query

https://gerrit.wikimedia.org/r/787088