Page MenuHomePhabricator

GlobalIdGenerator assumes tmp path is unique
Closed, ResolvedPublic

Description

MediaWiki's GlobalIdGenerator class creates two files in $wgTmpDirectory, i.e. mw-GlobalIdGenerator-UID-nodeid and mw-GlobalIdGenerator-UUID-128. The default value for $wgTmpDirectory is the system directory for temporary files (e.g. /tmp on Linux). When a system hosts multiple MediaWiki installations (for example on shared hosting) with the default settings, this causes trouble when one installation has already created /tmp/mw-GlobalIdGenerator-UID-nodeid and /tmp/mw-GlobalIdGenerator-UUID-128 and another installation attempts to access or create them. This is similar to T161453 .

One consequence of this, is that the VisualEditor returns a HTTP 500 error because of a permission error on the files in /tmp because another user has already created those files:

[2020-11-09 03:51:20.876788] [proxy_fcgi:error] [R:X6iup-BKHJAtBifWDmWMbQAAABQ] AH01071: Got error 'PHP message: PHP Warning:  fopen(/tmp/mw-GlobalIdGenerator-UUID-128): failed to open stream: Permission denied in /home/boss/thomasd/www/includes/libs/uuid/GlobalIdGenerator.php on line 449' web2

If one user hosts multiple MediaWiki installations, or if a shared hosting service would run installations from different users all as www-data, then one installation would be messing with the files of another.

There might be other consequences depending on where GlobalIdGenerator gets used, though I have only seen issues with VisualEditor so far. This does mean that it's impossible to use VisualEditor on shared hosting with the default settings.

My suggestion is to either ensure that the file names will be unique (such as by using a prefix), or by implementing T179901 as long as the created directory is unique per installation and not system-wide.

Event Timeline

I can confirm that indeed these files are created globally in /tmp , which is problematic whenever more than a single wiki is used. This could even be problematic for non-shared hosting where someone just happens to run two MediaWiki installations. It seems weird the regular functions for creating randomised or prefix temporary files weren't used in this case.

ppelberg moved this task from To Triage to Triaged on the VisualEditor board.
ppelberg added a subscriber: ppelberg.

hi @DanielsThomas, we appreciate you filing this ticket. Although, we think this is most likely a platform issue rather than a VisualEditor issue. As such, tagging that team.

I think it's correct for this file to be shared between different wikis, but we could put getmyuid() in the filename to avoid permission errors.

I think it's correct for this file to be shared between different wikis, but we could put getmyuid() in the filename to avoid permission errors.

I think that getmyuid() would absolutely make sense. I'm not sure if there's any risk of collision or race conditions when the file is shared between multiple wikis (of the same user), but if there's not I agree that adding getmyuid() to the file would be sufficient.

Change 808548 had a related patch set uploaded (by Thomas Daniels; author: Thomas Daniels):

[mediawiki/core@master] GlobalIdGenerator: include user id in file prefix

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

Change 808548 merged by jenkins-bot:

[mediawiki/core@master] GlobalIdGenerator: include user id in file prefix

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

tstarling claimed this task.