From bed19db954359043515300c995ebc40ebb97265a Mon Sep 17 00:00:00 2001 From: Damien Regad Date: Sat, 1 Nov 2014 19:45:47 +0100 Subject: [PATCH] XML Import: Fix php code injection vulnerability Egidio Romano discovered a vulnerability in the XML import plugin. User input passed through the "description" field (and the "issuelink" attribute) of the uploaded XML file isn't properly sanitized before being used in a call to the preg_replace() function which uses the 'e' modifier. This can be exploited to inject and execute arbitrary PHP code when the Import/Export plugin is installed. This fix is a partial backport from a master branch commit which has been confirmed as addressing the issue (84017535f8718685d755d58af7a39d80f52ffca8) excluding changes not relevant to fixing the security issue, including subsequent fixes (aea1a348043979e75a6cc021e4a0a7f8d3bb7211, 4350b4d4f0ee4fba423edcae1cd2117dc1e2d63b). Fixes #17725 (CVE-2014-7146) --- plugins/XmlImportExport/ImportXml.php | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/plugins/XmlImportExport/ImportXml.php b/plugins/XmlImportExport/ImportXml.php index 590f898..09ccc8d 100644 --- a/plugins/XmlImportExport/ImportXml.php +++ b/plugins/XmlImportExport/ImportXml.php @@ -102,16 +102,27 @@ public function import( ) { echo " Done\n"; - $importedIssues = $this->itemsMap_->getall( 'issue' ); - printf( "Processing cross-references for %s issues...", count( $importedIssues ) ); - foreach( $importedIssues as $oldId => $newId ) { - $bugData = bug_get( $newId, true ); - - $bugLinkRegexp = '/(^|[^\w])(' . preg_quote( $this->source_->issuelink, '/' ) . ')(\d+)\b/e'; - $replacement = '"\\1" . $this->getReplacementString( "\\2", "\\3" )'; + # replace bug references + $t_imported_issues = $this->itemsMap_->getall( 'issue' ); + printf( 'Processing cross-references for %s issues...', count( $t_imported_issues ) ); + foreach( $t_imported_issues as $t_old_id => $t_new_id ) { + $t_bug = bug_get( $t_new_id, true ); + $t_content_replaced = false; + $t_bug_link_regexp = '/(^|[^\w])(' . preg_quote( $this->source_->issuelink, '/' ) . ')(\d+)\b/'; + + # replace links in description + preg_match_all( $t_bug_link_regexp, $t_bug->description, $t_matches ); + if( is_array( $t_matches[3] ) && count( $t_matches[3] ) > 0 ) { + $t_content_replaced = true; + foreach ( $t_matches[3] as $t_old_id2 ) { + $t_bug->description = str_replace( $this->source_->issuelink . $t_old_id2, $this->getReplacementString( $this->source_->issuelink, $t_old_id2 ), $t_bug->description ); + } + } - $bugData->description = preg_replace( $bugLinkRegexp, $replacement, $bugData->description ); - $bugData->update( true, true ); + if( $t_content_replaced ) { + # only update bug if necessary (otherwise last update date would be unnecessarily overwritten) + $t_bug->update( true ); + } } echo " Done\n"; }