search

rokwire / safer-illinois-app

Source code repository of "Safer Illinois" App - the official COVID-19 app of the University of Illinois.
https://safer.illinois.edu/
Apache License 2.0
33 stars 23 forks source link

[BUG] Multiple Security Issues #687

Open VegaDeftwing opened 3 years ago

VegaDeftwing commented 3 years ago

Please see this blog post: https://soatok.blog/2021/08/17/safer-illinois-isnt/

VegaDeftwing commented 3 years ago

Hey there, it's been over two weeks since I've submitted this issue. Has this been looked into at all?

edthedev commented 3 years ago

Thanks for sharing this.

My team is investigating these concerns, and we will deliver a set of recommended follow up actions to the development team.

We will encourage the development team to do any follow up work here in GitHub where you and others can follow along with progress, and potentially collaborate.

soatok commented 3 years ago

Hi, I'm the initial reporter on this issue and will be happy to answer any questions you have about these findings

edthedev commented 3 years ago

@soatok Thanks!

edthedev commented 2 years ago

The team has completed analysis on these and started remediation. Thanks again for reporting these concerns!

soatok commented 2 years ago

@isaac-galvan I see you closed the issue, but I don't see any fixes in the develop branch or a pull request that addresses the problems reported,. The latest revision of e.g. Crypt.dart is from May 2021.

Am I overlooking something?

edthedev commented 2 years ago

@soatok @VegaDeftwing Since the report contains multiple issues, I believe the team is tracking them in separate issues.

soatok commented 2 years ago

@edthedev Thanks for clarifying.

Are these being tacked on Github or an external issue tracker?

If on Github, can you link the issues together (as simple as including #687 in the issue description)?

Do you have an estimated date for remediation? Failing that, do you have an estimated date for a date?

mihail-varbanov commented 2 years ago

Added security.md as per Recommendations for Safer Illinois - 2021 Sept (Safer.Illinois.Security.Report.pdf):

• https://github.com/rokwire/safer-illinois-app/commit/3d7c1d15ce14df68be98024f31053cd0901060e8

mihail-varbanov commented 2 years ago

Removed microBlinkScan API as per Recommendations for Safer Illinois - 2021 Sept (Safer.Illinois.Security.Report.pdf):

• https://github.com/rokwire/safer-illinois-app/issues/717 • https://github.com/rokwire/safer-illinois-app/commit/e0f6f29c156c28f146f1fe3edb8586aec50fe951 • https://github.com/rokwire/safer-illinois-app/commit/987ac6dda2d3619edbc5bee3575f721f5d38886e • https://github.com/rokwire/safer-illinois-app/commit/75c697aa3f0370e10130afa0745b09c62bbcb460

pmarkhennessy commented 2 years ago

The overall issues are being worked on and we will update this issue as we proceed.

soatok commented 2 years ago

There are some upstream issues in the Dart ecosystem that are worth paying attention to:

johnmpaul commented 2 years ago

Thank you for taking your time and expertise to inform us of issues you found in our Rokwire based Safer Illinois and Community apps and ecosystem.

We have had an extensive review by the University of Illinois Cyber Security team. We are following their considered recommendations and are almost complete with our work. Pointers to the changes in our source code are included in this GitHub issue. Upon completion of the work and necessary QA we will submit updated apps to Google and Apple.

After their approval and the release of the updated apps, we will post here the complete security recommendations for you and the Rokwire community to review.

We ask that any additional finding or issues you wish to raise, you do so via our secure communication channel as described in the new SECURITY.md files.

Thank you, Rokwire team.

VegaDeftwing commented 2 years ago

It took over three weeks for this issue to have any comment after first opening it.

Then, when work finally was started, there was no transparency as @edthedev closed the issue sating that remediation had been started before any relevant commits had been made, and stated that there were separate issues for each security issue, despite there not appearing to be any, and no indication given that an external issue tracker is being used. At this point, @soatok pointed this out, asking how or where the issues were being tracked only to be responded to with

The overall issues are being worked on and we will update this issue as we proceed.

from @pmarkhennessy , which, I mean, I'd sure hope so given that at that point it had been 70 days since the initial report.

Then, in a matter of one to two days two dramatic changes were made: one where exposure notification, which to me seems to be basically the entire point of the app, were removed rather than fixed and another where the dart encryption library seems to have been updated blindly in the hope that it would improve security - because if it's not fixed upstream, it's not our mistake right?

Yesterday, Soatok pointed out multiple issues in the underlying encryption library you're using, issues that there is absolutely no way have been addressed in that time, and now we're given a "Thank you" that really reads as "We really want this to go away, so please send this to a private email so that this may be shoved under the rug in the future"

And I get it, by opening this issue with a blog post publicly disclosing the vulns we made it clear that the apps were vulnerable for attackers to actually take advantage of it - hence your request for "responsible disclosure". But, then I must ask, where is the responsibility of the development team for this issue to have taken so long to address and then to be 'fixed' so hastily and by removing core functionality from an app that in every marketing place possible goes out of its way to claim being secured like Fort Knox. For issues to be this blatantly obvious it is honestly morbidly funny.

So, I really have to ask, what makes you think that you have earned the trust of the security community such that we would email securitysupport@illinois.edu when this issue felt like pulling teeth to get this far?

soatok commented 2 years ago

PointyCastle upstream has fixed the issue in 3.4.0-rc2 -- https://github.com/bcgit/pc-dart/issues/140#issuecomment-960519011 / https://github.com/bcgit/pc-dart/commit/bdf39f00ee3372a967176ff5f0ffa35dc3bce4bf

Thus, if you want to use the same encrypt API you've been using, once 3.4.0 is properly released, it will be safe to push on https://github.com/leocavalcante/encrypt/issues/23.

Regarding what @VegaDeftwing said earlier this week, I'm generally willing to play ball with disclosing issues privately, especially for code execution risks (e.g. XSS). As a security researcher, my obligation is to protect users. (This does NOT mean protecting vendors or their reputations.)

However, the response time and lack of transparency is burning trust and does not incentivize me to report things privately. If you'd like to correct course, there's no time like the present.