diff --git a/i18n/en.json b/i18n/en.json
index b9cb8c3ff..c8ce444c4 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -536,6 +536,7 @@
"abusefilter-diff-next": "Newer change",
"abusefilter-import-intro": "You can use this interface to import filters from other wikis.\nOn the source wiki, click \"{{int:abusefilter-edit-export}}\" under \"{{int:abusefilter-edit-tools}}\" on the editing interface.\nCopy from the textbox that appears, and paste it into this textbox, then click \"{{int:abusefilter-import-submit}}\".",
"abusefilter-import-submit": "Import data",
+ "abusefilter-import-invalid-data": "The data you tried to import is not valid",
"abusefilter-group-default": "Default",
"abusefilter-http-error": "An HTTP error occurred: $1.",
"abusefilter-view-privatedetails-submit": "View private details",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index bc094383c..09b26e29f 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -570,6 +570,7 @@
"abusefilter-diff-next": "Link to the diff view for the next change to this filter.\n\nSee also:\n* {{msg-mw|Abusefilter-diff-prev}}\n* {{msg-mw|Previousdiff}} and {{msg-mw|Nextdiff}}",
"abusefilter-import-intro": "{{doc-important|Do not translate {{int:abusefilter-edit-export}}, {{int:abusefilter-tools-subtitle}}, and {{int:abusefilter-import-submit}} unless you absolute must substitute any of them.}}\n\nRefers to:\n* {{msg-mw|Abusefilter-edit-export}}\n* {{msg-mw|Abusefilter-edit-tools}}\n* {{msg-mw|Abusefilter-import-submit}}",
"abusefilter-import-submit": "Used as label for the Submit button.\n\nPreceded by the textarea.\n\nUsed in:\n* {{msg-mw|Abusefilter-import-intro}}.",
+ "abusefilter-import-invalid-data": "Error message shown when provided data is invalid.",
"abusefilter-group-default": "The name for the default filter group. Most filters will be in this group.\n{{Identical|Default}}",
"abusefilter-http-error": "Error message for HTTP requests. Parameters:\n* $1 - HTTP response code.",
"abusefilter-view-privatedetails-submit": "Submit button label for viewing private details of an abuse log",
diff --git a/includes/AbuseFilter.php b/includes/AbuseFilter.php
index 662f1f625..a9964620d 100644
--- a/includes/AbuseFilter.php
+++ b/includes/AbuseFilter.php
@@ -641,6 +641,23 @@ class AbuseFilter {
return self::$filterCache[$filter];
}
+ /**
+ * Checks whether the given object represents a full abuse_filter DB row
+ * @param stdClass $row
+ * @return bool
+ */
+ public static function isFullAbuseFilterRow( stdClass $row ) {
+ $actual = array_keys( get_object_vars( $row ) );
+
+ if (
+ count( $actual ) !== count( self::ALL_ABUSE_FILTER_FIELDS )
+ || array_diff( self::ALL_ABUSE_FILTER_FIELDS, $actual )
+ ) {
+ return false;
+ }
+ return true;
+ }
+
/**
* Saves an abuse_filter row in cache
* @param string $id Filter ID (integer or "")
@@ -650,11 +667,7 @@ class AbuseFilter {
public static function cacheFilter( $id, $row ) {
// Check that all fields have been passed, otherwise using self::getFilter for this
// row will return partial data.
- $actual = array_keys( get_object_vars( $row ) );
-
- if ( count( $actual ) !== count( self::ALL_ABUSE_FILTER_FIELDS )
- || array_diff( self::ALL_ABUSE_FILTER_FIELDS, $actual )
- ) {
+ if ( !self::isFullAbuseFilterRow( $row ) ) {
throw new UnexpectedValueException( 'The specified row must be a full abuse_filter row.' );
}
self::$filterCache[$id] = $row;
diff --git a/includes/Views/AbuseFilterViewEdit.php b/includes/Views/AbuseFilterViewEdit.php
index 958f16c26..a03d7f3ca 100644
--- a/includes/Views/AbuseFilterViewEdit.php
+++ b/includes/Views/AbuseFilterViewEdit.php
@@ -87,7 +87,18 @@ class AbuseFilterViewEdit extends AbuseFilterView {
if ( $isImport || ( $request->wasPosted() && !$tokenMatches ) ) {
// Make sure to load from HTTP if the token doesn't match!
- $data = $this->loadRequest( $filter );
+ $status = $this->loadRequest( $filter );
+ if ( !$status->isGood() ) {
+ $out->addHTML(
+ Xml::tags(
+ 'p',
+ null,
+ Html::errorBox( $status->getMessage()->parse() )
+ )
+ );
+ return;
+ }
+ $data = $status->getValue();
} else {
$data = $this->loadFromDatabase( $filter, $history_id );
}
@@ -106,7 +117,12 @@ class AbuseFilterViewEdit extends AbuseFilterView {
*/
private function saveCurrentFilter( $filter, $history_id ) : void {
$out = $this->getOutput();
- list( $newRow, $actions ) = $this->loadRequest( $filter );
+ $reqStatus = $this->loadRequest( $filter );
+ if ( !$reqStatus->isGood() ) {
+ // In the current implementation, this cannot happen.
+ throw new LogicException( 'Should always be able to retrieve data for saving' );
+ }
+ list( $newRow, $actions ) = $reqStatus->getValue();
$dbw = wfGetDB( DB_MASTER );
$status = AbuseFilter::saveFilter( $this, $filter, $newRow, $actions, $dbw );
@@ -1168,9 +1184,10 @@ class AbuseFilterViewEdit extends AbuseFilterView {
* @throws BadMethodCallException If called without the request being POSTed or when trying
* to import a filter but $filter is not 'new'
* @param int|string $filter The filter ID being requested.
- * @return array
+ * @return Status If good, the value is the array [ row, actions ]. If not, it contains an
+ * error message.
*/
- public function loadRequest( $filter ): array {
+ public function loadRequest( $filter ): Status {
$request = $this->getRequest();
if ( !$request->wasPosted() ) {
// Sanity
@@ -1197,6 +1214,10 @@ class AbuseFilterViewEdit extends AbuseFilterView {
}
$data = FormatJson::decode( $import );
+ if ( !$this->isValidImportData( $data ) ) {
+ return Status::newFatal( 'abusefilter-import-invalid-data' );
+ }
+
$importRow = $data->row;
$actions = wfObjectToArray( $data->actions );
@@ -1309,7 +1330,7 @@ class AbuseFilterViewEdit extends AbuseFilterView {
$row->af_actions = implode( ',', array_keys( $actions ) );
- return [ $row, $actions ];
+ return Status::newGood( [ $row, $actions ] );
}
/**
@@ -1348,4 +1369,42 @@ class AbuseFilterViewEdit extends AbuseFilterView {
$this->getConfig()->get( 'AbuseFilterDefaultDisallowMessage' )
);
}
+
+ /**
+ * Perform basic validation on the JSON-decoded import data. This doesn't check if parameters
+ * are valid etc., but only if the shape of the object is right.
+ *
+ * @param mixed $data Already JSON-decoded
+ * @return bool
+ */
+ private function isValidImportData( $data ) {
+ global $wgAbuseFilterActions;
+
+ if ( !is_object( $data ) ) {
+ return false;
+ }
+
+ $arr = get_object_vars( $data );
+
+ $expectedKeys = [ 'row' => true, 'actions' => true ];
+ if ( count( $arr ) !== count( $expectedKeys ) || array_diff_key( $arr, $expectedKeys ) ) {
+ return false;
+ }
+
+ if ( !is_object( $arr['row'] ) || !is_object( $arr['actions'] ) ) {
+ return false;
+ }
+
+ foreach ( $arr['actions'] as $action => $params ) {
+ if ( !array_key_exists( $action, $wgAbuseFilterActions ) || !is_array( $params ) ) {
+ return false;
+ }
+ }
+
+ if ( !AbuseFilter::isFullAbuseFilterRow( $arr['row'] ) ) {
+ return false;
+ }
+
+ return true;
+ }
}
diff --git a/includes/Views/AbuseFilterViewImport.php b/includes/Views/AbuseFilterViewImport.php
index a6d147d39..80dc8c98e 100644
--- a/includes/Views/AbuseFilterViewImport.php
+++ b/includes/Views/AbuseFilterViewImport.php
@@ -17,6 +17,7 @@ class AbuseFilterViewImport extends AbuseFilterView {
$formDescriptor = [
'ImportText' => [
'type' => 'textarea',
+ 'required' => true
]
];
HTMLForm::factory( 'ooui', $formDescriptor, $this->getContext() )
diff --git a/tests/phpunit/AbuseFilterSaveTest.php b/tests/phpunit/AbuseFilterSaveTest.php
index 31d7b1241..73919b537 100644
--- a/tests/phpunit/AbuseFilterSaveTest.php
+++ b/tests/phpunit/AbuseFilterSaveTest.php
@@ -209,7 +209,11 @@ class AbuseFilterSaveTest extends MediaWikiTestCase {
$existing = isset( $args['testData']['existing'] );
$viewEdit = $this->getViewEdit( $user, $params, $existing );
- list( $newRow, $actions ) = $viewEdit->loadRequest( $filter );
+ $reqStatus = $viewEdit->loadRequest( $filter );
+ if ( !$reqStatus->isGood() ) {
+ $this->fail( 'Cannot retrieve request data correctly' );
+ }
+ list( $newRow, $actions ) = $reqStatus->getValue();
/** @var IDatabase|MockObject $dbw */
$dbw = $this->getMockForAbstractClass( IDatabase::class );