Page MenuHomePhabricator
Files F32358406

07-T260485.patch
Reedy (Sam Reed)
Actions

Authored By
Reedy
Sep 21 2020, 10:49 PM
Size
2 KB
Referenced Files
None
Subscribers
None

07-T260485.patch
View Options

From e95b7202e16f77c02edaaa58354d769badf4db67 Mon Sep 17 00:00:00 2001
From: Martin Urbanec <martin.urbanec@wikimedia.cz>
Date: Thu, 20 Aug 2020 21:51:51 +0200
Subject: [PATCH] SECURITY: ActorMigration: Load user from the correct database
In ActorMigration::getInsertValues, when creating a User object, calling
User::getActorId triggers a call to User::load, which ignores
the database passed to getInsertValues, meaning incorrect actor IDs
are returned.
To ensure that the correct (foreign) database is used, try
to get the actor ID from the correct database within ActorMigration
service, and if that fails, let User class handle the actor ID creation.
Todo notes are left in the patch to fix the issue properly,
by making User object wiki-aware.
Bug: T260485
Change-Id: Iaa886a1824e5a74f4501ca7e28917c780222aac0
---
includes/ActorMigration.php | 32 ++++++++++++++++++++++++++++----
1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/includes/ActorMigration.php b/includes/ActorMigration.php
index c2ad1cdb8c..192e0409c6 100644
--- a/includes/ActorMigration.php
+++ b/includes/ActorMigration.php
@@ -239,6 +239,23 @@ class ActorMigration {
return $this->joinCache[$key];
}
+ /**
+ * Get actor ID from UserIdentity, if it exists
+ */
+ private function getExistingActorId( IDatabase $dbw, UserIdentity $user ) {
+ $row = $dbw->selectRow(
+ 'actor',
+ [ 'actor_id' ],
+ [ 'actor_name' => $user->getName() ],
+ __METHOD__
+ );
+ if ( $row === false ) {
+ return false;
+ }
+
+ return $row->actor_id;
+ }
+
/**
* Get UPDATE fields for the actor
*
@@ -262,11 +279,18 @@ class ActorMigration {
$ret[$text] = $user->getName();
}
if ( $this->stage & SCHEMA_COMPAT_WRITE_NEW ) {
- // We need to be able to assign an actor ID if none exists
- if ( !$user instanceof User && !$user->getActorId() ) {
- $user = User::newFromAnyId( $user->getId(), $user->getName(), null );
+ // UBN fix for T260485 - do not let User object to handle existing actor IDs
+ // TODO: Make User object wiki-aware and let it handle all cases.
+ $existingActorId = $this->getExistingActorId( $dbw, $user );
+ if ( $existingActorId !== false ) {
+ $ret[$actor] = $existingActorId;
+ } else {
+ // We need to be able to assign an actor ID if none exists
+ if ( !$user instanceof User && !$user->getActorId() ) {
+ $user = User::newFromAnyId( $user->getId(), $user->getName(), null );
+ }
+ $ret[$actor] = $user->getActorId( $dbw );
}
- $ret[$actor] = $user->getActorId( $dbw );
}
return $ret;
}
--
2.20.1

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
8510864
Default Alt Text
07-T260485.patch (2 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits