meshenger-android icon indicating copy to clipboard operation
meshenger-android copied to clipboard

Revoking permission at BackupActivity causes app crashed

Open aper-project opened this issue 4 years ago • 2 comments

Hi, there, I've found a crash in Meshenger v3.0.3, which is downloaded from F-Droid.

I seems that it is related to the dangerous permission android.permission.READ_EXTERNAL_STORAGE and the following call chain:

com.github.isabsent.filepicker.SimpleFilePickerDialog.onStart()void
  com.github.isabsent.filepicker.SimpleFilePickerDialog.isExternalStorageRoot(java.lang.String)boolean
    android.os.Environment.getExternalStorageDirectory()java.io.File

This call chain does not involve either permission checking (via checkSelfPermission) nor permission request (via requestPermissions) to the corresponding permission, which may produce unexpected consequence.

Reproduce crash

In fact, I found this lack of permission management causes a crash on my Samsung device, following the steps:

  1. grant the storage permission for the app (in system setting)
  2. in the app, click "Backup" at the top-right menu and enter the BackupActivity
  3. go to system setting and revoke the storage permission
  4. tap "BACK" button, then the app crashes

And here is the trace:

04-29 14:31:40.855 23821-23821/? E/AndroidRuntime: FATAL EXCEPTION: main
    Process: d.d.meshenger, PID: 23821
    java.lang.NullPointerException: Attempt to read from field 'java.util.ArrayList d.d.meshenger.Database.contacts' on a null object reference
        at d.d.meshenger.MainService$MainBinder.getContactsCopy(MainService.java:394)
        at d.d.meshenger.MainService$MainBinder.pingContacts(MainService.java:378)
        at d.d.meshenger.MainActivity.onServiceConnected(MainActivity.java:86)
        at android.app.LoadedApk$ServiceDispatcher.doConnected(LoadedApk.java:1335)
        at android.app.LoadedApk$ServiceDispatcher$RunConnection.run(LoadedApk.java:1352)
        at android.os.Handler.handleCallback(Handler.java:739)
        at android.os.Handler.dispatchMessage(Handler.java:95)
        at android.os.Looper.loop(Looper.java:158)
        at android.app.ActivityThread.main(ActivityThread.java:7225)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1230)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1120)

@mwarning Could you help me review this issue? If you need more information, I may make a video showing the crash. Very thanks!

aper-project avatar Apr 29 '21 06:04 aper-project

hi @aper-project, thank you for the report and details analysis! Since the crash involves editing the apps permissions, I am hestitant to put effort into fixing this in the current master branch.

Also, the is there is a meshenger rewrite in progress that does not utilize READ_EXTERNAL_STORAGE. Progress is going forward in phases: https://github.com/meshenger-app/meshenger-android/commits/development (this is my wild west development branch)

mwarning avatar Apr 29 '21 16:04 mwarning

If you want to fix it yourself, I can give you some help at least.

mwarning avatar Apr 29 '21 16:04 mwarning

The backup activity code has been rewritten. This problem does not seem to occur anymore.

mwarning avatar Oct 09 '22 16:10 mwarning