Page MenuHomePhabricator

Phan SecurityCheck-XSS and SecurityCheck-SQLInjection errors in SecurePoll extension
Closed, ResolvedPublic

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?

Event Timeline

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 added a project: Security-Team.
Daimona assigned this task to Bawolff.
Daimona edited projects, added: phan-taint-check-plugin; removed: phan.
Daimona subscribed.

The suppression is currently working, nothing actionable here.