Page MenuHomePhabricator

security review of Wikimetrics
Closed, ResolvedPublic

Description

Have a security review of wikimetrics once we're ready for it.

UPDATE: review done, most issues are relatively quick and easy to fix

Event Timeline

ggellerman raised the priority of this task from to Medium.
ggellerman updated the task description. (Show Details)
ggellerman changed Security from none to None.

(Once wikimetrics is "ready" for the security review, please assign to csteipp.)

Krenair renamed this task from Arrange for security review to Arrange for wikimetrics security review.Dec 14 2014, 12:09 AM
kevinator renamed this task from Arrange for wikimetrics security review to security review of Wikimetrics {dove}.Feb 4 2015, 6:02 PM
kevinator lowered the priority of this task from Medium to Low.
kevinator updated the task description. (Show Details)
kevinator edited projects, added Analytics-Kanban; removed Analytics-Engineering.
kevinator moved this task from Next Up to Tasked_Hidden on the Analytics-Kanban board.

Umm, so this was filed in 2014. What is wikimetrics? Is it something that (still) needs a security review?

Reedy changed the task status from Open to Stalled.Sep 11 2018, 7:44 PM

Umm, so this was filed in 2014. What is wikimetrics? Is it something that (still) needs a security review?

@charlotteportero @Bawolff

Yeah, pretty sure @Aklapper is correct and this is https://metrics.wmflabs.org/. I'm not sure if still based on https://github.com/rfaulkner/wikipedia_user_metrics or if it's a rewrite but seems likely that's the source origin. It runs on Cloud VPS for now and even has an appropriate privacy policy so that's nice. One issue I do have with it at the moment is that http://metrics.wikimedia.org/ redirects to https://metrics.wmflabs.org/. This is in violation of long standing policy to never redirect from a realm of high security to one of low security. i.e. if you get something on Cloud Services when you were led you to believe it was a *.wikimedia.org property that's improper. I'm looking to see if that's written down anywhere but it's been a long standing position of the Labs/Cloud Services team. I'm surfacing this issue in another place soon so I'm not concerned about it being addressed in this context. It may be useful for someone to lookover the oauth code since this is treated semi-official at least, but my guess is not high priority.

Hey all-

I was going to take a first crack at this. It probably makes sense to reach out to Dan as a first step to confirm the status of the tool. Seems to still be actively used/developed - last commit in May 2018: https://gerrit.wikimedia.org/g/analytics/wikimetrics/+/refs/heads/master. Github repo seems way out-of-date, so I'm guessing that's not really mirrored. It appears Wikimetrics replaced User_Metrics, which seems completely dead according to https://www.mediawiki.org/wiki/User_Metrics, along with the gerrit repo and pydoc on stat1.wikimedia.org no longer existing. I was going to focus on SA and general code-quality stuff. Looks like it's been Dockerized, which should aid in analysis/testing. Also might make sense to look at the deploy repo: https://github.com/wikimedia/analytics-wikimetrics-deploy

Got a response email from Dan/milimetric, tracking here:

  1. The tool is still actively used (metrics.wmflabs.org) - seems to be, just wanted to confirm.

Yes, but it's a very very low priority tool for our team (we work on at least 10 other projects that are higher priority). So if you have other more pressing reviews to do, keep this in mind. The userbase is very small, and the data it's accessing is all 100% public (since it runs on the cloud with only access to the public databases).

  1. That master would be a good branch to work from: https://gerrit.wikimedia.org/g/analytics/wikimetrics/+/refs/heads/master

Yep, that's the latest code.

I’d basically plan to get the Docker dev env spun up and run varying linter/sa tools against the code, poke around for config issues and maybe attempt some pen/dos attacks.

There are some problems getting the code working, as nobody's looked at it in a while. We filed some tasks but since it's low priority they're just sitting there: https://phabricator.wikimedia.org/T193783, https://phabricator.wikimedia.org/T193780

Docker fixes @ https://phabricator.wikimedia.org/T193780 (w/ https://gerrit.wikimedia.org/r/464059/), though flake8 tests failing, unrelated to my patch :/

Next: sec report template creation, doc, automated tests (might be gnarly) and manual code review.

sbassett set Security to Software security bug.Oct 19 2018, 6:42 PM
sbassett added a project: acl*security.
sbassett changed the visibility from "Public (No Login Required)" to "Custom Policy".

Posting security review with vulnerabilities for a live labs app.

Security Review Summary - October 2018
Overall, the current wikimetrics code (https://gerrit.wikimedia.org/g/analytics/wikimetrics/+/refs/heads/master) seems to be in good shape, even 4 years post-launch :) Many good things - a solid web framework (Flask) leveraging abstraction layers (Jinja for html templates, SqlAlchemy for ORM, etc.) and clean code. I've listed some items I found during my security review of the app - there's nothing I would consider critical, especially for a 4-year-old app behind oauth which apparently does not receive much traffic. Please let me know if there are any questions or if I can provide more clarity regarding any issues.

Vulnerable Python Packages
As reported by safety via requirements.txt:

  1. flask, installed 0.10.1, affected <0.12.3, id 36388 flask version Before 0.12.3 contains a CWE-20: Improper Input Validation vulnerability in flask that can result in Large amount of memory usage possibly leading to denial of service. This attack appear to be exploitable via Attacker provides JSON data in incorrect encoding. This vulnerability appears to have been fixed in 0.12.3.
  2. pyjwt, installed 0.2.1, affected <1.0.0, id 26040 pyjwt before 1.0.0 allows to bypass signature verification by setting the alg header to None.
  3. pyjwt, installed 0.2.1, affected <1.5.1, id 35014 In PyJWT 1.5.0 and below the invalid_strings check in HMACAlgorithm.prepare_key does not account for all PEM encoded public keys. Specifically, the PKCS1 PEM encoded format would be allowed because it is prefaced with the string -----BEGIN RSA PUBLIC KEY----- which is not accounted for. This enables symmetric/asymmetric key confusion attacks against users using the PKCS1 PEM encoded public keys, which would allow an attacker to craft JWTs from scratch.
  4. httplib2, installed 0.9, affected <=0.9.2, id 25848 httplib2 before and including 0.9.2 on "SSL certificate hostname mismatch" it is checked only once: https://github.com/httplib2/httplib2/issues/5
  5. oauthlib, installed 0.6.3, affected <0.7.0, id 25909 oauthlib before 0.7.0 is not stripping client provided passwords from OAuth2 logs.
  6. requests, installed 2.3.0, affected <2.6.0, id 26102 requests 2.6.0 fixes handling of cookies on redirect. Previously a cookie without a host value set would use the hostname for the redirected URL exposing requests users to session fixation attacks and potentially cookie stealing.
  7. requests, installed 2.3.0, affected >=2.1,<=2.5.3, id 26103 The resolve_redirects function in sessions.py in requests 2.1.0 through 2.5.3 allows remote attackers to conduct session fixation attacks via a cookie without a host value in a redirect.
  8. python-mwoauth Older version detected (current 0.3.2) but no Github-reported issues, CVEs, Snyk or safety db.

Vulnerable JavaScript Libraries
As found via https://nvd.nist.gov/, https://snyk.io/vuln, etc:

  1. jquery 1.9.1
  2. jquery 1.9.1
  3. Bootstrap 2.3.2
    • CVE-2018-14040 - In Bootstrap before 4.1.2, XSS is possible in the collapse data-parent attribute
    • CVE-2018-14041 - In Bootstrap before 4.1.2, XSS is possible in the data-target property of scrollspy.
    • CVE-2018-14042 - In Bootstrap before 4.1.2, XSS is possible in the data-container property of tooltip.
  4. Moment 2.1.0
  5. Knockout < 3.5.0-beta, <3.0.0 >=2.1.0-pre

Security Headers
Like many mw-related apps, wikimetrics doesn't fare very well here. Not a major issue, but could be iteratively improved upon within future wikimetrics releases: https://securityheaders.com/?q=https%3A%2F%2Fmetrics.wmflabs.org%2F&followRedirects=on

Python-Taint Findings
python-taint found untrusted user-data making its way unscathed to some database calls within wikimetrics/wikimetrics/controllers/demo.py. This would be fairly low risk since the two relevant endpoints and supplemental code can only be reached within the app's DEBUG mode, though it would be good to ensure these variables are properly-cleaned for a future wikimetrics release:

  1. variable n set on line 22, executed on line 42
  2. variable n set on line 22, executed on line 69
  3. variable project set on line 22, executed on line 69

Backup/Temporary Files
Should be harmless, but could be cleaned up or archived:

  1. wikimetrics/models/validate_cohort.py.bak
  2. wikimetrics/api/file_manager.py.bak
  3. wikimetrics/api/cohorts.py.bak
  4. wikimetrics/controllers/reports.py.bak
  5. wikimetrics/controllers/cohorts.py.bak
  6. tests/test_models/test_cohort.py.bak
  7. tests/test_metrics/test_namespace_edits.py.bak
  8. database_migrations/versions/35adbe20f3d_add_column_raw_id_or_name_to_wiki_user.py.bak

HTTP Leaks
A few HTTP leaks from external js, though I understand this app falls under a different pp than other mw projects, so this may not be an issue:

  1. wikimetrics/templates/csv_upload.html - 26:<script src="//ajax.aspnetcdn.com/ajax/jquery.validate/1.11.1/jquery.validate.min.js"></script>
  2. wikimetrics/templates/program_metrics_reports.html - 7:<script src="//ajax.aspnetcdn.com/ajax/jquery.validate/1.11.1/jquery.validate.min.js"></script>
  3. wikimetrics/templates/csv_upload_review.html - 86:<script src="//ajax.aspnetcdn.com/ajax/jquery.validate/1.11.1/jquery.validate.min.js"></script>

Potential DoS With File Uploads
For the application route /cohorts/upload, I was able to reveal some basic signs of a potential DoS when working with larger files. I created some large data files with random bytes via dd and attempted to upload these files via my local Docker instance of wikimetrics. I noticed some general performance issues and errors after submission. I then attempted to submit a large data file (~ 300 Mb) against https://metrics.wmflabs.org/cohorts/upload and was able to temporarily exhaust nginx resources. Setting app.config['MAX_CONTENT_LENGTH'] within wikimetrics/configurables.py catches large upload files early and throws an exception.

Potential CSRF Issues
As noted within a couple of TODOs within the code, CSRF or some other mechanism could be implement within various JS file upload libraries to harden the app within:

  1. wikimetrics/static/js/reportList.js
  2. wikimetrics/static/js/reportCreate.js

Supplemental Materials
Please also find attached the output from a handful of Python-specific analysis tools I ran against wikimetrics. Where possible I attempted to configure these tools for certainty and specifically targeting performance and security issues:

  1. bandit - wikimetrics-bandit-report-2018.txt
  2. pylint (only a small subset of perf/sec checks run) - wikimetrics-pylint-report-2018.txt
  3. python-taint (diff with minor linting issues which pyt auto-refactors) - wikimetrics-pyt-linting-report-2018.txt
  4. safety - wikimetrics-safety-report-2018.txt
  5. vulture - wikimetrics-vulture-report-2018.txt
Milimetric renamed this task from security review of Wikimetrics {dove} to security review of Wikimetrics.Oct 19 2018, 8:13 PM
Milimetric raised the priority of this task from Low to Medium.
Milimetric moved this task from Deprioritized to Operational Excellence on the Analytics board.
Milimetric updated the task description. (Show Details)
sbassett lowered the priority of this task from Medium to Lowest.
sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".