search

mikifus / padland

Padland is a tool to manage, share, remember and read collaborative documents based on the Etherpad technology in Android.
Apache License 2.0
63 stars 15 forks source link

[Bug] Potential crashes in PadViewActivity #69

Closed LightningRS closed 1 year ago

LightningRS commented 1 year ago

Description: We inspected the latest release version of Padland (1.9 from F-droid) using an Android application testing tool under development, and found some points that may cause crash.

NullPointerException in PadViewActivity._makePadUrl()

In function PadViewActivity._makePadUrl(), line 154 in PadViewActivity.java, lifecycle method finish() is called to stop the activity since check of url in PadData failed.

    if( ! WhiteListMatcher.checkValidUrl(PadData.getUrl()) ) {
        Toast.makeText(this, getString(R.string.padview_toast_invalid_url), Toast.LENGTH_SHORT).show();
        finish();   // <===== To stop the activity
    }

However, calling of finish() cannot skip the following instructions, the app will continue to call WhiteListMatcher.isValidHost() to check the host of the url. This check also apparently fails, so Uri.parse(PadData.getUrl() on line 167 will still be executed.

    if( ! WhiteListMatcher.checkValidUrl(PadData.getUrl()) ) {
        Toast.makeText(this, getString(R.string.padview_toast_invalid_url), Toast.LENGTH_SHORT).show();
        finish();   // <===== The app will continue to run the following code
    }
    if( ! WhiteListMatcher.isValidHost(PadData.getUrl(), getServerWhiteList()) ) {
        // The check is apparently fails
        ...
        Intent i = new Intent(Intent.ACTION_VIEW);
        ...
        i.setData(Uri.parse(PadData.getUrl()));  // <===== This line will be executed
        ...

According to other code, the return value of PadData.getUrl() may be null, which will cause NullPointerException when calling as Uri.parse(null). Thus, The purpose of function WhiteListMatcher.checkValidUrl() to check the url is not fully achieved.

Therefore, we changed the return type of the functino PadViewActivity._makePadUrl() from void to boolean, which will return a "flag" representing successfully parsed PadUrl or not. Then, in lifecycle method onCreate(), we will check the flag and call finish() and return; to completely stop the activity.

    if (!this._makePadUrl()) {
        finish();
        return;
    }

Related stacktrace:

E AndroidRuntime: java.lang.RuntimeException: Unable to start activity ComponentInfo{com.mikifus.padland/com.mikifus.padland.PadViewActivity}: java.lang.NullPointerException: uriString
E AndroidRuntime:       at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2698)
E AndroidRuntime:       at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2759)
E AndroidRuntime:       at android.app.ActivityThread.-wrap12(ActivityThread.java)
E AndroidRuntime:       at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1482)
E AndroidRuntime:       at android.os.Handler.dispatchMessage(Handler.java:102)
E AndroidRuntime:       at android.os.Looper.loop(Looper.java:154)
E AndroidRuntime:       at android.app.ActivityThread.main(ActivityThread.java:6190)
E AndroidRuntime:       at java.lang.reflect.Method.invoke(Native Method)
E AndroidRuntime:       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:892)
E AndroidRuntime: Caused by: java.lang.NullPointerException: uriString
E AndroidRuntime:       at android.net.Uri$StringUri.<init>(Uri.java:479)
E AndroidRuntime:       at android.net.Uri$StringUri.<init>(Uri.java)
E AndroidRuntime:       at android.net.Uri.parse(Uri.java:441)
E AndroidRuntime:       at com.mikifus.padland.PadViewActivity.d(SourceFile:12)
E AndroidRuntime:       at com.mikifus.padland.PadViewActivity.onCreate(SourceFile:5)
E AndroidRuntime:       at android.app.Activity.performCreate(Activity.java:6701)
E AndroidRuntime:       at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1118)
E AndroidRuntime:       at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2651)

NullPointerException in PadViewActivity._getPadId()

In fucntion PadViewActivity._getPadId(), line 242-243 in PadViewActivity.java, PadData.getUrl() has been called without checking whether PadData is null, which may produce NullPointerException.

    Pad PadData = this._getPadDataFromIntent(); // _getPadDataFromIntent() may return null
    Cursor c = padlistDb._getPadByUrl(PadData.getUrl());  // PadData unchecked

Therefore, we provide the following fix according to original code.

    Pad PadData = this._getPadDataFromIntent();
    if (PadData == null) {
        return 0;
    }
    Cursor c = padlistDb._getPadByUrl(PadData.getUrl());

Related stacktrace:

E AndroidRuntime: java.lang.RuntimeException: Unable to start activity ComponentInfo{com.mikifus.padland/com.mikifus.padland.PadViewActivity}: java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String com.mikifus.padland.Models.Pad.getUrl()' on a null object reference
E AndroidRuntime:       at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2698)
E AndroidRuntime:       at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2759)
E AndroidRuntime:       at android.app.ActivityThread.-wrap12(ActivityThread.java)
E AndroidRuntime:       at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1482)
E AndroidRuntime:       at android.os.Handler.dispatchMessage(Handler.java:102)
E AndroidRuntime:       at android.os.Looper.loop(Looper.java:154)
E AndroidRuntime:       at android.app.ActivityThread.main(ActivityThread.java:6190)
E AndroidRuntime:       at java.lang.reflect.Method.invoke(Native Method)
E AndroidRuntime:       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:782)
E AndroidRuntime: Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String com.mikifus.padland.Models.Pad.getUrl()' on a null object reference
E AndroidRuntime:       at com.mikifus.padland.PadViewActivity._getPadId(PadViewActivity.java:244)
E AndroidRuntime:       at com.mikifus.padland.PadViewActivity._getPadData(PadViewActivity.java:184)
E AndroidRuntime:       at com.mikifus.padland.PadViewActivity._makePadUrl(PadViewActivity.java:153)
E AndroidRuntime:       at com.mikifus.padland.PadViewActivity.onCreate(PadViewActivity.java:86)
E AndroidRuntime:       at android.app.Activity.performCreate(Activity.java:6701)
E AndroidRuntime:       at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1118)
E AndroidRuntime:       at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2651)

StringIndexOutOfBoundsException in PadViewActivity.makePadData()

In fucntion PadViewActivity.makePadData(), line 530-531 in PadViewActivity.java, PadData.substring() has been called without checking the index value returned by padUrl.lastIndexOf(), which may produce StringIndexOutOfBoundsException when padUrl is not well-formed.

    padName = padUrl.substring(padUrl.lastIndexOf("/") + 1);
    padServer = padUrl.substring(0, padUrl.lastIndexOf("/"));

Since this part of code is to parse the input parameters, we wrap this part of code with try-catch statements in order to catch all potential exceptions, and finally return null. It can significantly reduce the risk of the app crashing here.

    try {
        if (padUrl == null || padUrl.isEmpty()) {
        ...
    } catch (Exception e) {
        // Any exception is related to the format error of parameters
        e.printStackTrace();
        return null;
    }

Related stacktrace:

E AndroidRuntime: java.lang.RuntimeException: Unable to start activity ComponentInfo{com.mikifus.padland/com.mikifus.padland.PadViewActivity}: java.lang.StringIndexOutOfBoundsException: length=19; regionStart=0; regionLength=-1
E AndroidRuntime:       at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2698)
E AndroidRuntime:       at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2759)
E AndroidRuntime:       at android.app.ActivityThread.-wrap12(ActivityThread.java)
E AndroidRuntime:       at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1482)
E AndroidRuntime:       at android.os.Handler.dispatchMessage(Handler.java:102)
E AndroidRuntime:       at android.os.Looper.loop(Looper.java:154)
E AndroidRuntime:       at android.app.ActivityThread.main(ActivityThread.java:6190)
E AndroidRuntime:       at java.lang.reflect.Method.invoke(Native Method)
E AndroidRuntime:       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:892)
E AndroidRuntime:       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:782)
E AndroidRuntime: Caused by: java.lang.StringIndexOutOfBoundsException: length=19; regionStart=0; regionLength=-1
E AndroidRuntime:       at java.lang.String.substring(String.java:1931)
E AndroidRuntime:       at com.mikifus.padland.PadViewActivity.makePadData(PadViewActivity.java:535)
E AndroidRuntime:       at com.mikifus.padland.PadViewActivity._getPadDataFromIntent(PadViewActivity.java:224)
E AndroidRuntime:       at com.mikifus.padland.PadViewActivity._getPadId(PadViewActivity.java:243)
E AndroidRuntime:       at com.mikifus.padland.PadViewActivity._getPadData(PadViewActivity.java:184)
E AndroidRuntime:       at com.mikifus.padland.PadViewActivity._makePadUrl(PadViewActivity.java:153)
E AndroidRuntime:       at com.mikifus.padland.PadViewActivity.onCreate(PadViewActivity.java:86)
E AndroidRuntime:       at android.app.Activity.performCreate(Activity.java:6701)
E AndroidRuntime:       at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1118)
E AndroidRuntime:       at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2651)

Version Info:

mikifus commented 1 year ago

Sorry for taking to long to check this, it is merged.