I am getting some file path related issues with SecurityCheck-OTHER, there are from the new version.

This is expected, see https://gerrit.wikimedia.org/r/c/mediawiki/tools/phan/SecurityCheckPlugin/+/609754.

They should be checked one by one, suppressed if false positives, or fixed if not.

I am not sure how to escape them properly, if needed.

It's not really possible to escape a tainted path, so let's hope these are false positives.

includes\filerepo\file\LocalFileRestoreBatch.php:181 SecurityCheck-OTHER Calling method \RepoGroup::getFileProps() in \LocalFileRestoreBatch::execute that outputs using tainted argument #1 ($deletedUrl). (Caused by: includes\filerepo\RepoGroup.php +460) (Caused by: includes\filerepo\file\LocalFileRestoreBatch.php +160; includes\filerepo\file\LocalFileRestoreBatch.php +158; includes\filerepo\file\LocalFileRestoreBatch.php +142; includes\filerepo\file\LocalFileRestoreBatch.php +125)

I guess we can consider file paths safe, and suppress this one.

includes\installer\DatabaseInstaller.php:231 SecurityCheck-OTHER Calling method \Wikimedia\Rdbms\Database::sourceFile() in \DatabaseInstaller::stepApplySourceFile that outputs using tainted argument #1. (Caused by: includes\libs\rdbms\database\Database.php +5000) (Caused by: includes\installer\DatabaseInstaller.php +193; includes\Storage\PageUpdater.php +1135; includes\Storage\PageUpdater.php +1080; includes\Storage\PageUpdater.php +1300; includes\Storage\PageUpdater.php +1236; includes\FileDeleteForm.php +243; includes\filerepo\FileRepo.php +1257; includes\Storage\PageUpdater.php +1135; includes\Storage\PageUpdater.php +1080; includes\Storage\PageUpdater.php +1300; includes\Storage\PageUpdater.php +1236; includes\FileDeleteForm.php +243; ...)

I don't really want to investigate this one, because taint-check certainly cannot understand a dynamic callable (call_user_func( [ $this, $sourceFileMethod ]), so let's suppress.

includes\installer\Installer.php:1349 SecurityCheck-OTHER Calling method \opendir() in \Installer::findExtensionsByType that outputs using tainted argument #1 ($extDir). (Caused by: includes\installer\Installer.php +1344)

Taint-check doesn't know that $IP is safe, and Installer is also a peculiar use case, this is suppressable.

includes\upload\UploadFromFile.php:56 SecurityCheck-OTHER Calling method \UploadFromFile::initializePathInfo() in \UploadFromFile::initialize that outputs using tainted argument #2. (Caused by: includes\WebRequestUpload.php +101; includes\WebRequestUpload.php +45)

It complains about $_FILES['tmp_name'] which is theoretically safe, since PHP automatically creates a temporary file, and this only be changed via php.ini. Suppress.

maintenance\compareParsers.php:163 SecurityCheck-OTHER Calling method \file_put_contents() in \CompareParsers::processRevision that outputs using tainted argument #1. (Caused by: includes\Title.php +1858; includes\Title.php +1856; includes\Title.php +1855)

file_put_contents(
	$this->saveFailed . '/' . rawurlencode( $title->getPrefixedText() ) . ".txt",
	$text
);

This is safe because '/' is forbidden in titles, and rawurlencode() should remove any special characters. Otherwise, titles with "bad" names could indeed cause some unwanted file to get written. So not exploitable, also because someone would have to run the script on a prod machine. Suppress.

maintenance\preprocessorFuzzTest.php:78 SecurityCheck-OTHER Calling method \file_put_contents() in \PPFuzzTester::execute that outputs using tainted argument #1. (Caused by: maintenance\preprocessorFuzzTest.php +77)
maintenance\preprocessorFuzzTest.php:79 SecurityCheck-OTHER Calling method \file_put_contents() in \PPFuzzTester::execute that outputs using tainted argument #1. (Caused by: maintenance\preprocessorFuzzTest.php +77)

Can be suppressed, md5() should be considered safe, and this is also not really exploitable.

maintenance\renderDump.php:115 SecurityCheck-OTHER Calling method \file_put_contents() in \DumpRenderer::handleRevision that outputs using tainted argument #1 ($filename). (Caused by: maintenance\renderDump.php +102)

Another Title::getPrefixedText(). It's guarded with rawurlencode, hence suppressable.

Change 652553 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] build: Enable phan-taint-check-plugin and suppress issues

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

(FTR, the two issues with rawurlencode were fixed by hardcoding the function as removing path taint; the ones with $_FILES are going to be fixed with https://gerrit.wikimedia.org/r/c/mediawiki/tools/phan/SecurityCheckPlugin/+/650828)

Change 652553 merged by jenkins-bot:
[mediawiki/core@master] build: Enable phan-taint-check-plugin and suppress issues

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