Page MenuHomePhabricator

Security Readiness Review For mapbox-gl-rtl-text
Open, Stalled, LowestPublic

Description

Project Information

Description of the tool/project:
An Emscripten port of a subset of the functionality of International Components for Unicode (ICU) necessary for Mapbox GL JS to support right to left text rendering. Supports the Arabic and Hebrew languages, which are written right-to-left. Mapbox Studio loads this plugin by default.

Although the library is officially supported by mapbox and works with Mapbox GL JS, we will need a fork of it for maplibre-gl-js that will be reviewed as well at T274356: Security Readiness Review For maplibre-gl-js

Description of how the tool will be used at WMF:
This plugin shall be used in Maps (Kartographer) extension, preliminary work is being experimented at https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Kartographer/+/663867

Dependencies

List dependencies, or upstream projects that this project relies on.

The library only points to mapbox-gl as a peer dependency, but we will be using a fork of mapbox-gl called maplibre-gl as mentioned before.

"peerDependencies": {
    "mapbox-gl": ">=0.32.1 <2.0.0"
  },

Has this project been reviewed before?

Please link to tasks or wiki pages of previous reviews.

No.

Working test environment

Please link or describe setup process for setting up a test environment.

TBD

Post-deployment

Name of team responsible for tool/project after deployment and primary contact.

Product-Infrastructure-Team-Backlog

Event Timeline

MSantos changed the task status from Open to Stalled.Apr 20 2021, 10:09 AM
MSantos created this task.
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Security Review Summary - Mapbox-gl-rtl-text - 2021-05-13

Overall, the current vendor code under consideration has an overall risk rating of: medium.
This project is not updated and has a lot of open vulnerabilities. However, since user data isn't collected and there don't seem to be other privacy concerns, I think usability for RTL languages is worth the medium risk. If possible, we should try to do the necessary updates and possibly help with maintenance of this project.

General Security Information

Statistic/InfoValueRisk
Repositoryhttps://github.com/mapbox/mapbox-gl-rtl-text none
Relevant tag/branchmain none
Last commit reviewed (if relevant)046c611 medium
Recent contributions to code (6 months)0 high
Active developers with > 10 commits1 high
Current overall usage41 stars, 15 forks medium
Current open security issues1 medium
Methods for reporting security issuesnone medium

Vulnerable Packages
The tap package is extremely out of date, it's currently at version 5.8 and the latest version is at 15.0
It's only a dev dependency, but npm audit returned 54 vulnerabilities, including 17 high and 2 critical,
all from dependencies of the npm tap package
Similar to npm audit, snyk also found 8 high vulnerabilities from Istanbul, which is a dependency of tap
I think this is a medium risk, despite it being a dev package, there are too many open issues

Outdated Packages
As reported via npm outdated:
None reported

General Security Issues

  1. There's an open issue about the build script that has yet to be addressed by the project maintainers - https://github.com/mapbox/mapbox-gl-rtl-text/pull/26
  2. There's only 1 maintainer with over 10 commits and this project hasn't been updated in two years
  3. As mentioned above, the tap package is out of date and has over 50 listed vulnerabilities. It's only a dev dependency, but with that many vulnerabilities it's really a risk
  4. There is no package-lock.json file in the repo

@MSantos this is medium risk if it is still going to be used in a production capacity. In that case, we either need a mitigation plan to address the security issues or your manager/tech lead needs to accept the risk, which means that this project will be added to the WMF risk registry.

Thank you @Mstyles for doing the review. Just FYI, before thinking about accepting the risk we will be considering the following steps:

This should keep stalled for now.

sbassett triaged this task as Lowest priority.Jun 22 2021, 3:52 PM
sbassett moved this task from Waiting to Back Orders on the secscrum board.
sbassett added a subscriber: sbassett.

Per @MSantos above - setting to lowest, stalled and back ordered for now, until we have a clearer idea on if this library will be used in Wikimedia production and if risk will be accepted or mitigated by the ostensible maintainers.

sbassett moved this task from Waiting to Back Orders on the secscrum board.
sbassett added a subscriber: Mstyles.

@MSantos - Does it look like this library will eventually be used within Wikimedia production? Or should we decline this task for now? Thanks.