Page MenuHomePhabricator
Create Task
Maniphest T294441

ULS CI Blocked: includes/api/ApiULSSetLanguage.php:75 SecurityCheck-SQLInjection
Closed, ResolvedPublic
Actions

Description

CI fails for ULS and is blocking patches from being merged. See: https://integration.wikimedia.org/ci/job/mwext-php72-phan-docker/145858/console

Full error message:

13:13:37 includes/api/ApiULSSetLanguage.php:75 SecurityCheck-SQLInjection Calling method \MediaWiki\User\UserOptionsManager::setOption() in \ApiULSSetLanguage::execute that outputs using tainted argument #3 (`$languageCode`). (Caused by: ../../includes/user/UserOptionsManager.php +465; ../../includes/user/UserOptionsManager.php +204; Builtin-\Wikimedia\Rdbms\IDatabase::insert; ../../includes/user/UserOptionsManager.php +436; ../../includes/user/UserOptionsManager.php +428; ../../includes/user/UserOptionsManager.php +420; ../../includes/user/UserOptionsManager.php +204; ../../includes/language/LanguageCode.php +162; ../../includes/user/UserOptionsManager.php +565; ../../includes/user/UserOptionsManager.php +564; ../../includes/user/UserOptionsManager.php +540; ../../includes/user/UserOptionsManager.php +539; ...) (Caused by: includes/api/ApiULSSetLanguage.php +50; Builtin-\WebRequest::getText)

Same error appears for the following patches:

  1. 734888: Use HookHandlers | https://gerrit.wikimedia.org/r/c/mediawiki/extensions/UniversalLanguageSelector/+/734888
  2. 734766: MW 1.35: Re-add class attribute for ULS | https://gerrit.wikimedia.org/r/c/mediawiki/extensions/UniversalLanguageSelector/+/734766

Details

SubjectRepoBranchLines +/-
Suppress taint-check false positive blocking CImediawiki/extensions/UniversalLanguageSelectormaster+1 -0
Customize query in gerrit

Related Objects

Event Timeline

abi_ triaged this task as Unbreak Now! priority.Oct 27 2021, 3:13 PM
abi_ created this task.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 27 2021, 3:13 PM
Daimona claimed this task.Oct 27 2021, 3:40 PM
Daimona edited projects, added phan-taint-check-plugin; removed phan.Oct 27 2021, 3:42 PM

This is a bug in taint-check: T290563. UserOptionsManager inserts multiple rows at once, which is not correctly special-cased in the already-special-cased handling of DB functions. I'm going to suppress the issue in ULS.

gerritbot added a comment.Oct 27 2021, 3:45 PM

Change 735011 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/UniversalLanguageSelector@master] Suppress taint-check false positive blocking CI

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

gerritbot added a project: Patch-For-Review.Oct 27 2021, 3:45 PM
gerritbot added a comment.Oct 27 2021, 4:11 PM

Change 735011 merged by jenkins-bot:

[mediawiki/extensions/UniversalLanguageSelector@master] Suppress taint-check false positive blocking CI

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

Daimona closed this task as Resolved.Oct 27 2021, 4:15 PM
Daimona removed a project: Patch-For-Review.
ReleaseTaggerBot added a project: MW-1.38-notes (1.38.0-wmf.7; 2021-11-02).Oct 27 2021, 5:00 PM
abi_ added a comment.Oct 27 2021, 5:31 PM

Thanks for the quick response on this one.

Daimona moved this task from Backlog to Issues in MediaWiki code on the phan-taint-check-plugin board.Oct 15 2022, 4:55 PM
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