markusfisch / BinaryEye

Yet another barcode scanner for Android
https://play.google.com/store/apps/details?id=de.markusfisch.android.binaryeye
MIT License
1.38k stars 114 forks source link

SSID with length greater 32 crashes BinaryEye #402

Closed I-Al-Istannen closed 10 months ago

I-Al-Istannen commented 10 months ago

Hey,

a bit of a weird bug report, but this was encountered in the wild (in a capture-the-flag :P). It seems like 7913f09089785cdc252968f3ff4eb07d6e04ed72 introduced a max-length validation (32) for SSIDs. This was later reverted in be6d62222a4f145c0f1c2f258b235715b8045c51.

Sadly, my android 13 on my fairphone apparently uses AOSP between those two commits. This leads to an IllegalArgumentException in the call to WifiNetworkSuggestion$Builder.setSsid. This is probably done here: https://github.com/markusfisch/BinaryEye/blob/92fe4dd55fc6f521cec204804bf126bb465186f1/app/src/main/kotlin/de/markusfisch/android/binaryeye/actions/wifi/WifiConnector.kt#L160 and called from the WifiAction#canExecuteOn.

I am not sure what the best fix would be (as you likely do not want to introduce your own validation), but maybe you can catch the error and not completely crash BinaryEye when it occurs.

A crashing QR code is image


The stacktrace from android is not too helpful, but you can find it here anyways ``` Process: de.markusfisch.android.binaryeye, PID: 4832 java.lang.IllegalArgumentException: Max SSID length is 32 bytes, but received 35 bytes! at android.net.wifi.WifiSsid.(WifiSsid.java:54) at android.net.wifi.WifiSsid.fromUtf8Text(WifiSsid.java:92) at android.net.wifi.WifiNetworkSuggestion$Builder.setSsid(WifiNetworkSuggestion.java:293) at h0.b.a(Unknown Source:0) at h0.m.d(Unknown Source:4) at h0.m.c(Unknown Source:0) at h0.m.h(Unknown Source:52) at h0.a.d(Unknown Source:16) at x.a.a(Unknown Source:24) at o0.n.E1(Unknown Source:10) at o0.n.K1(Unknown Source:13) at o0.n.c0(Unknown Source:352) at android.support.v4.app.t.C0(Unknown Source:7) at android.support.v4.app.b0.C0(Unknown Source:501) at android.support.v4.app.b0.z0(Unknown Source:38) at android.support.v4.app.b0.A0(Unknown Source:49) at android.support.v4.app.n.n(Unknown Source:162) at android.support.v4.app.b0.f0(Unknown Source:38) at android.support.v4.app.b0.g0(Unknown Source:109) at android.support.v4.app.b0.E0(Unknown Source:88) at android.support.v4.app.b0.e0(Unknown Source:21) at android.support.v4.app.w.x(Unknown Source:4) at android.support.v4.app.u.onStart(Unknown Source:32) at n.b.onStart(Unknown Source:0) at android.app.Instrumentation.callActivityOnStart(Instrumentation.java:1544) at android.app.Activity.performStart(Activity.java:8330) at android.app.ActivityThread.handleStartActivity(ActivityThread.java:3671) at android.app.servertransaction.TransactionExecutor.performLifecycleSequence(TransactionExecutor.java:221) at android.app.servertransaction.TransactionExecutor.cycleToPath(TransactionExecutor.java:201) at android.app.servertransaction.TransactionExecutor.executeLifecycleState(TransactionExecutor.java:173) at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:97) at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2308) at android.os.Handler.dispatchMessage(Handler.java:106) at android.os.Looper.loopOnce(Looper.java:201) at android.os.Looper.loop(Looper.java:288) at android.app.ActivityThread.main(ActivityThread.java:7918) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:942) ```
markusfisch commented 10 months ago

Thanks a lot for reporting this! And also for suggesting a solution! 👍 Weird thing for sure!

I'm catching the IllegalArgumentException now, just as you suggested. I don't see a better way than this, too.

I-Al-Istannen commented 10 months ago

I tested the master branch and it now indeed no longer crashes BinaryEye :) Thank you for the quick fix!

I looked into the IEEE 802.11 standard and it says

The length of the SSID information field is between 0 and 32 octets.

So I get where the person was coming from, but reality does not necessarily agree with pesky standards… :P

markusfisch commented 10 months ago

Thanks a lot for testing it right away! 👍 I'm just piling up a few other changes and then I will make a new release.

So I get where the person was coming from, but reality does not necessarily agree with pesky standards… :P

That's true 😉