Page MenuHomePhabricator

Security Readiness Review For Citoid VE Mobile ISBN Barcode Scanner
Open, MediumPublic

Description

Project Information

Description of the tool/project:
In the mobile visual editor, it will be possible to cite physical books by scanning the book's ISBN barcode.

Description of how the tool will be used at WMF:
In the mobile visual editor, it will be possible to cite physical books by scanning the book's ISBN barcode.

Dependencies
None

Has this project been reviewed before?
No

Working test environment
https://patchdemo.wmflabs.org/wikis/cbe37578751b8297c6f29ebdd4406bb7/w/index.php?title=Test&mobileaction=toggle_view_mobile#/editor/all
(Switch to visual editor, then start the citation workflow by clicking on the double quote icon)

Post-deployment
Editing Team @ppelberg

Event Timeline

Restricted Application added a project: secscrum. · View Herald TranscriptNov 30 2020, 3:34 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Hey @matmarex can you fill in the

Link to code repository / patchset:

and

Working test environment

Portion of this ticket

Esanders updated the task description. (Show Details)Nov 30 2020, 4:10 PM
Esanders updated the task description. (Show Details)Nov 30 2020, 4:13 PM
Reedy renamed this task from Security Readiness Review For {...} to Security Readiness Review For Citoid VE Mobile ISBN Barcode Scanner.Nov 30 2020, 4:27 PM
Esanders updated the task description. (Show Details)Nov 30 2020, 4:31 PM

I removed references to the Android app, as there exists no patch or plans to implement this outside of mobile web.

sbassett moved this task from Incoming to Back Orders on the secscrum board.Nov 30 2020, 9:35 PM
sbassett reassigned this task from matmarex to Reedy.Dec 2 2020, 5:15 PM
sbassett moved this task from Back Orders to In Progress on the secscrum board.
Reedy added a comment.Dec 2 2020, 5:15 PM

https://serratus.github.io/quaggaJS/

Code is https://github.com/serratus/quaggaJS/blob/v0.12.1/lib/quagga.js

I note https://github.com/serratus/quaggaJS/releases/tag/v0.12.1 isn't very... comforting

This is just a patch release for those who are interested in testing it with the new Safari Technology Preview. So far it works on iOS (11 Beta) and macOS, with some limitations: The video is upside-down on iOS devices. Haven't figured out why. Have fun testing!

Last release 7 Jun 2017. 14 commits to master since this release

Also, 18 months since any commits on https://github.com/serratus/quaggaJS/commits/master

If it didn't work on iOS 11 beta... Does it work on 11 RTM? 12? 13? 14?

I just spotted https://github.com/serratus/quaggaJS/issues/444

Stopped working after upgrade to Safari 14.0 and iOS 14

If it's not working on the current default browser on the current stable release of iOS... This doesn't sound helpful or productive for anyone to be deployed in its current state :)

187 open issues in https://github.com/serratus/quaggaJS/issues. 11 PR open in https://github.com/serratus/quaggaJS/pulls

The developer doesn't seem to have been active very recently at all in this repo

With this cursory look at the library, it seems this will likely end up as a "High" risk rating. For more information on that, see T249039#6309061

Esanders added a comment.EditedDec 2 2020, 5:51 PM

In my last few rounds of testing product everything was working fine on iOS. I'll test iOS 14 and if it is broken we can disable in those browsers. However these are all product issues, not security issues.

Update: I just tested this on iOS 14 and it worked fine

11 PR open in https://github.com/serratus/quaggaJS/pulls

PRs have been merged as recently as April 2019. I don't a lack of very recent active development points to any specific security issues.

@Reedy, we appreciate you having an initial look at this work. A resulting question for you (I think): What – if any – further review is required before the Security team is able to assign this work a risk level?

Two notes:

Jcross triaged this task as Medium priority.Tue, Feb 9, 9:21 PM

@Jcross Thanks for taking a look at this! I saw you added a priority, which is a handy coincidence after I had a chat with @ppelberg about this task today. Given your activity here, I assume you're aware there is an open question remaining. Our esteemed colleagues on the Editing-team would value this review happening sometime in Q4. Given your team's shorthandedness ATM, does that seem like a reasonable expectation? :)

Hi @MBinder_WMF - I noted that we still had it marked "Needs Triage" and wanted to correct that error. Adjust priority as needed. There is discussion happening around the remaining question and it was raised again just yesterday. We'll pursue resolution for you as soon as possible and apologize for the delay.