Page MenuHomePhabricator

Phan SecurityCheck-XSS and SecurityCheck-SQLInjection errors in SecurePoll extension
Open, HighPublic

Description

Could be a false alarm, but phan is throwing a bunch of warnings via its jenkins tests. See:

https://integration.wikimedia.org/ci/job/mwext-php70-phan-seccheck-docker/21888/console

triggered by

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

(that particular gerrit patch doesn't seem to have anything to do with the warnings being emitted, it's just the first patch submitted to SecurePoll for a while?)

Most of these seem harmless: they are taint warnings on cli scripts.

But one seems mildly concerning:

15:18:17   <file name="./auth-api.php">
15:18:17     <error line="33" severity="warning" message="Calling method \out() in [no method] that outputs using tainted argument $[arg #1]. (Caused by: ./auth-api.php +19)" source="SecurityCheck-XSS"/>
15:18:17     <error line="39" severity="warning" message="Calling method \out() in [no method] that outputs using tainted argument $[arg #1]. (Caused by: ./auth-api.php +19)" source="SecurityCheck-XSS"/>
15:18:17     <error line="44" severity="warning" message="Calling method \out() in [no method] that outputs using tainted argument $[arg #1]. (Caused by: ./auth-api.php +19)" source="SecurityCheck-XSS"/>
15:18:17     <error line="50" severity="warning" message="Calling method \out() in [no method] that outputs using tainted argument $[arg #1]. (Caused by: ./auth-api.php +19)" source="SecurityCheck-XSS"/>
15:18:17   </file>

Perhaps this should be investigated by someone who knows SecurePoll and/or the purpose of the phan checks better than I do?

Details

Related Gerrit Patches:

Event Timeline

cscott created this task.Oct 17 2018, 4:17 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 17 2018, 4:17 PM
cscott added a subscriber: Reedy.Oct 17 2018, 4:26 PM
Ladsgroup merged a task: Restricted Task.Nov 15 2018, 11:26 AM
Ladsgroup added subscribers: Ladsgroup, Bawolff, tstarling.
Bawolff changed the visibility from "Custom Policy" to "Public (No Login Required)".Nov 15 2018, 12:12 PM

The api-auth one is likely because the mime type is not text/html but the script still thinks it is (Thought i already suppressed that)

Change 473724 had a related patch set uploaded (by Brian Wolff; owner: Brian Wolff):
[integration/config@master] There are some issues with seccheck on SecurePoll move to non-voting.

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

Change 473724 merged by jenkins-bot:
[integration/config@master] There are some issues with seccheck on SecurePoll move to non-voting.

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

chasemp triaged this task as High priority.Dec 9 2019, 4:45 PM
chasemp added a project: Security-Team.
Restricted Application added a project: Core Platform Team. · View Herald TranscriptDec 9 2019, 4:45 PM
chasemp moved this task from Incoming to Back Orders on the Security-Team board.Dec 9 2019, 5:29 PM