AllanWang / Frost-for-Facebook

An extensive and functional third party app for Facebook
https://allanwang.github.io/Frost-for-Facebook/
GNU General Public License v3.0
1.1k stars 85 forks source link

Bugsnag still connects even if analytics are disabled #1491

Closed adolfintel closed 5 years ago

adolfintel commented 5 years ago

Describe the bug I noticed that the app connects to Bugsnag even with analytics disabled. Care to explain that?

To Reproduce Steps to reproduce the behaviour:

  1. Run Frost
  2. Sniff phone traffic from another device

Details (please provide at least the app version):

IzzySoft commented 5 years ago

Preferred solution would be an fdroid build variant that comes entirely without Bugsnag (or drop it in general). Bugsnag is listed at Exodus being a tracker – especially because it does not ask the user's consent to upload data. If you really need crash reports etc., maybe take a look at ACRA instead, which by default creates the report and asks the user to confirm before it is send. Another possible option would be CrashReporter, depending on what options you need. These two candidates are not listed as "Trackers".

adolfintel commented 5 years ago

I agree, the app isn't even GDPR compliant because of this tracker.

IzzySoft commented 5 years ago

Exactly. As it currently stands, we'll have to add the Tracking AntiFeature at F-Droid. I'll probably do that in a few hours (when I'm back at my machine at home). Of course that step can be skipped if @AllanWang also agrees with us and announces a "fix" to be tagged short-term (if it takes a bit, I'd add the AntiFeature today and remove it as soon as the issue is solved by having a "tracker-less" version at F-Droid).

AllanWang commented 5 years ago

I'm willing to switch to another crash reporter if one is recommended by F-Droid (like ACRA as mentioned above). From my experience, it becomes difficult to manage bugs if they aren't submitted automatically. When I initially submitted Frost, I had chosen to switch from Crashlytics to Bugsnag to make the app compliant to F-Droid, as described in the PR. Maybe something was overlooked, but I did not intend on adding anything without explicit declaration.

In terms of needs, I predominantly use the tracker to find critical errors, ie those that occur thousands of times shortly after a release. One problem is potentially adding the proguard conversions to ensure that reports are readable, and I will take a look at the other options to see if they support it

AllanWang commented 5 years ago

I also wonder if Bugsnag's disableExceptionHandler function is enough to stop all auto reporting.

adolfintel commented 5 years ago

if(Prefs.analytics) initBugSnag() should be enough to fix it.

But to be GDPR compliant you have to inform the user that you're using analytics BEFORE doing that, or have them be opt-in (recommended).

AllanWang commented 5 years ago

That would not allow for toggling without restarting the app. Making the pref default to false should be straightforward

adolfintel commented 5 years ago

So you want to keep bugsnag but make it opt-in? I think that would be a reasonable compromise.

AllanWang commented 5 years ago

@adolfintel @IzzySoft to preface, I still intend on making the changes above.

There are mentions of breaching GDPR compliance, but from what I've reviewed recently, it applies purely to personal data correct? Is submitting "crumbs" of constant strings, unrelated to the user, a breach of GDPR? Even bugsnag mentions how to make reports compliant.

On the Gitlab thread, there is mention that the analytics pref only toggles a single logger line. That line also happens to be the only "automatic" report that produces user related data, ie the video url. That was the mindset I had going into this project initially. I'd naturally have to scan the app more throughly again, but almost all of the user related logs are already disabled for non debug builds.

adolfintel commented 5 years ago

I'd have to look through the GDPR more thoroughly but in my opinion, using bugsnag without telling the user (or worse, against explicit consent) is a breach because even though it doesn't send explicit identifiers like my email address, it does send stuff that can be easily used to deanonymize users, such as the wifi network it is connected to (there are public databases that give you the physical location of a wifi network with a given ID). I don't know if you can see this data or if they keep it to themselves.

IzzySoft commented 5 years ago

What @adolfintel wrote. Basically, you send that to a 3rd party service, enabling that to track the user. Regardless how few data you may include (and with Bugsnag being open source, one could check if it sticks to the rules) – in order to do that you at least reveal the users' IP addresses, which AFAIK already count as "personal data". If in addition it really sends the WiFi network names or other data completely unrelated to the crash report, even more so.

AllanWang commented 5 years ago

Took a while but I've submitted a new release.

I had to spend my time to address potential phishing warnings (#1504).

I also posted in that PR about potentially adding auto updates back to F-Droid. At first, it was blocked because I generate version codes and names programmatically, but they are directly in relation to the github tags

IzzySoft commented 5 years ago

I generate version codes and names programmatically, but they are directly in relation to the github tags

Doesn't help much that they are in direct relation. The way our update checker works is by simple RegEx-parsing of the gradle file to check not only versionName (which is part of your tag name), but also versionCode (to make sure before the build that this really is an update, i.e. versionCode is higher than what we already have). That doesn't work if you use function calls or variable names there. So we cannot enable auto-updates for your app currently.

AllanWang commented 5 years ago

@IzzySoft what if I add a comment on top?

// versionCode 20302000
versionCode androidGitVersion.code()
// versionName v2.3.2
versionName androidGitVersion.name()

And I will only change this when I make a new tag

IzzySoft commented 5 years ago

It's comments that will be ignored? If you can add them on top that way, just remove the other two lines and the //, and we are fine :laughing: Honestly: if you can add them there literally, you can do so with the real ones as well.

AllanWang commented 5 years ago

Well it's if your regex will just find the first instance. Git version will not only generate by tag, but will also add commit hashes if it is not an exact version, which is why I need it. I build test builds for each commit and I'd like to know which ones people are using if they need to post bugs.

Does your regex care about comments?

IzzySoft commented 5 years ago

TBH, I cannot give exact details on that (I'm not a packager, and have zero build experience). That question is better addressed to our packagers. I'm mainly dealing with metadata (such as descriptions, links etc).

mbeko commented 3 years ago

@adolfintel @IzzySoft As I see it, the issue originated from this audit and the fixing PR has been merged as well as the associated F-Droid release.

Yet, the anti-feature is still being displayed. Is there a particular reason for this or can I ignore it because it doesn't apply anymore?

IzzySoft commented 3 years ago

@mbeko if you're sure it has been fixed, feel free opening an issue/MR to have the AntiFeature removed. Supply references, ideally "reproducible proof" – like the PR removed the tracking library, that can be checked; which was the last version affected and which the first unaffected.

mbeko commented 3 years ago

Thanks, @IzzySoft ! I understand that I should open the issue or MR in the F-Droid GitLab.

I don't have a GitLab account, though, and I'm a bit reluctant to create one just for this issue. I'm also not familiar with F-Droid's contribution processes and the four pages of subprojects are a bit overwhelming.

Therefore, I'll post the references here for now. Would it be OK for you to copy them to the proper place in the F-Droid GitLab? I'm posting the raw Markdown at the bottom for easy transfer.


In the latest version (2.4.7) of Frost, the tracking library Bugsnag is not used. The relevant code fragments have been commented out:

These changes were introduced with commit 9d0feeed that is first contained in version 2.4.6.

So 2.4.5 was the version where Bugsnag was still used.


In the latest version (2.4.7) of Frost, the tracking library Bugsnag is not used. The relevant code fragments have been commented out:
* [Initialisation in `FrostApp.kt`](https://github.com/AllanWang/Frost-for-Facebook/blob/v2.4.7/app/src/main/kotlin/com/pitchedapps/frost/FrostApp.kt#L74)
* Configuration upon analytics preference change in [`CorePrefs.kt`](https://github.com/AllanWang/Frost-for-Facebook/blob/v2.4.7/app/src/main/kotlin/com/pitchedapps/frost/prefs/sections/CorePrefs.kt#L97-L105) and [`OldPrefs.kt`](https://github.com/AllanWang/Frost-for-Facebook/blob/v2.4.7/app/src/main/kotlin/com/pitchedapps/frost/prefs/OldPrefs.kt#L115-L123)
* [Logging configuration in `L.kt`](https://github.com/AllanWang/Frost-for-Facebook/blob/v2.4.7/app/src/main/kotlin/com/pitchedapps/frost/utils/L.kt#L65-L70)

These changes were introduced with [commit `9d0feeed`](https://github.com/AllanWang/Frost-for-Facebook/commit/9d0feeed6a6aa4e88a115e06778f215faaefc3a8) that is first contained in version 2.4.6.

So 2.4.5 was the version where Bugsnag was still used.
AllanWang commented 3 years ago

Thanks for the replies. Moving to a new opened issue