ikarus23 / MifareClassicTool

An Android NFC app for reading, writing, analyzing, etc. MIFARE Classic RFID tags.
http://www.icaria.de/mct/
GNU General Public License v3.0
4.71k stars 907 forks source link

Importing / exporting individual dumps corrupts data [LG Volt / Android 4.4.2] #445

Closed twisteroidambassador closed 10 months ago

twisteroidambassador commented 12 months ago

App version: 4.1.0, running on LG Volt LS740 with Android 4.4.2.

When exporting an individual dump as a .mct file, the resulting file seems to have its line order mixed up. When importing the same file, a toast message is shown: "Error: Some block(s) contains invalid data (not hex)".

If the same dump is exported as any other format, then the exported file seems correct. Importing the exported files again, the import process completes without error, but attempting to open the dump via "Edit/Analyze Dump File" will show two toast messages: "Error: Some block(s) contains invalid data (not hex)" and "Error: Data was not correct for this editor".

The zip file attached below contains exported files, and part of a "Backup / Export Everything" dump:

mct-export-debug.zip

MifareClassicTool\dump-files\sample-UID_93787FF7_2023-11-14_14-48-52.mct is the original dump, generated by tapping the save icon in "Read Tag". The contents look correct.

This dump is exported in different file formats, and the files can be seen in exports\ . The .mct file is corrupt, while other formats look correct.

The exported files are renamed and then imported back into MCT, which generates these files: MifareClassicTool\dump-files\sample-reimport-*.mct. All of these are corrupt, and the file contents are identical to the initially exported .mct file.

ikarus23 commented 11 months ago

Thank you for the detailed report! Not sure what could cause the corrupted export. Looks like some race condition, but I have to take a closer look into the code.

Is this behavior new? Or did you never try the export functions on older versions of MCT?

twisteroidambassador commented 11 months ago

I don't think I have tried individual exports on older versions, so I can't say whether this is new.

ikarus23 commented 10 months ago

So far I was not able to reproduce it or to find the issue.

ikarus23 commented 10 months ago

Hmmm, no idea what could cause this. I feel like it must be device specific. Or maybe the old Android version? Anyways, without debugging it on your device, I have no idea on how to tackle this down.

twisteroidambassador commented 10 months ago

I went and tested it on older app versions. 3.2.0 export .mct files correctly, while 4.0.0 exports corrupted files. Perhaps this has something to do with the new storage model introduced in 4.0.0.

Would there be a way for I to set up the phone for some kind of remote debugging?

ikarus23 commented 10 months ago

There are a lot of changes between 3.2.0 and 4.0.0. Was there even an export option in 3.2.0? I only think there was a "share" option without a file format converting option.

Although the Android Debugging Bridge supports TCP network, I'm not sure if there is a good way to do it remotely. Probably using some kind of VPN or other ready to use service. Or just something like TeamViewer to a PC with the Device connected and Android Studio installed.

twisteroidambassador commented 10 months ago

So I looked at the code, and my guess for the culprit is this:

https://github.com/ikarus23/MifareClassicTool/blob/addf57cc45f364f41b5ce34ca9c897e1d8032b62/Mifare%20Classic%20Tool/app/src/main/java/de/syss/MifareClassicTool/Activities/ImportExportTool.java#L652

convertDump always use JSON format as intermediary, and when converting from JSON to MCT, JSONObject.keys iterator is used to iterate through the blocks, while for BIN and EML formats a for loop is used.

According to https://developer.android.com/reference/org/json/JSONObject#keys() :

The order of the keys is undefined.

So, perhaps on older Android versions, the order is really arbitrary, while on later versions it's either order-preserving or sorted.

What's more, the corrupted export having multiple repeating Sector headers is consistent with the following code:

https://github.com/ikarus23/MifareClassicTool/blob/addf57cc45f364f41b5ce34ca9c897e1d8032b62/Mifare%20Classic%20Tool/app/src/main/java/de/syss/MifareClassicTool/Activities/ImportExportTool.java#L658-L660

where a new sector header is added whenever the sector number of the current line is different from the last line.

Disclaimer: I have very little Java and no Android dev experience, so the above could be totally wrong.

ikarus23 commented 10 months ago

I'm sorry for the pain you must have felt when you went through that code. For me it also is just a hobby. But to your point: The catch with the iterator sounds very promising! I will have a closer look. Thanks!

ikarus23 commented 10 months ago

Ok, I've implemented a fix. Can you try this testing version?

twisteroidambassador commented 10 months ago

No pain no gain. ;-)

Unfortunately the test version just dies right away with a message "Unfortunately, MIFARE Classic Tool has stopped". I got the following logcat:

E/ThemeUtils( 8324): View class androidx.appcompat.widget.AppCompatTextView is an AppCompat widget that can only be used with a Theme.AppCompat theme (or descendant).
E/ThemeUtils( 8324): View class androidx.appcompat.widget.AppCompatTextView is an AppCompat widget that can only be used with a Theme.AppCompat theme (or descendant).
D/AndroidRuntime( 8324): Shutting down VM
W/dalvikvm( 8324): threadid=1: thread exiting with uncaught exception (group=0x41768e48)
E/AndroidRuntime( 8324): FATAL EXCEPTION: main
E/AndroidRuntime( 8324): Process: de.syss.MifareClassicTool, PID: 8324
E/AndroidRuntime( 8324): java.lang.RuntimeException: Unable to start activity ComponentInfo{de.syss.MifareClassicTool/de.syss.MifareClassicTool.Activities.MainMenu}: java.lang.IllegalStateException: You need to use a Theme.AppCompat theme (or descendant) with this activity.
E/AndroidRuntime( 8324):        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2202)
E/AndroidRuntime( 8324):        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2252)
E/AndroidRuntime( 8324):        at android.app.ActivityThread.access$800(ActivityThread.java:139)
E/AndroidRuntime( 8324):        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1200)
E/AndroidRuntime( 8324):        at android.os.Handler.dispatchMessage(Handler.java:102)
E/AndroidRuntime( 8324):        at android.os.Looper.loop(Looper.java:136)
E/AndroidRuntime( 8324):        at android.app.ActivityThread.main(ActivityThread.java:5103)
E/AndroidRuntime( 8324):        at java.lang.reflect.Method.invokeNative(Native Method)
E/AndroidRuntime( 8324):        at java.lang.reflect.Method.invoke(Method.java:515)
E/AndroidRuntime( 8324):        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:790)
E/AndroidRuntime( 8324):        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:606)
E/AndroidRuntime( 8324):        at dalvik.system.NativeStart.main(Native Method)
E/AndroidRuntime( 8324): Caused by: java.lang.IllegalStateException: You need to use a Theme.AppCompat theme (or descendant) with this activity.
E/AndroidRuntime( 8324):        at androidx.appcompat.app.AppCompatDelegateImpl.createSubDecor(AppCompatDelegateImpl.java:926)
E/AndroidRuntime( 8324):        at androidx.appcompat.app.AppCompatDelegateImpl.ensureSubDecor(AppCompatDelegateImpl.java:889)
E/AndroidRuntime( 8324):        at androidx.appcompat.app.AppCompatDelegateImpl.setContentView(AppCompatDelegateImpl.java:772)
E/AndroidRuntime( 8324):        at androidx.appcompat.app.AppCompatActivity.setContentView(AppCompatActivity.java:197)
E/AndroidRuntime( 8324):        at de.syss.MifareClassicTool.Activities.MainMenu.onCreate(MainMenu.java:111)
E/AndroidRuntime( 8324):        at android.app.Activity.performCreate(Activity.java:5275)
E/AndroidRuntime( 8324):        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1087)
E/AndroidRuntime( 8324):        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2166)
E/AndroidRuntime( 8324):        ... 11 more
W/ActivityManager( 1009):   Force finishing activity de.syss.MifareClassicTool/.Activities.MainMenu
ikarus23 commented 10 months ago

Thanks. That's caused by a completely different change. Good to know that it will break older devices. :) Otherwise I had crashed a lot of old devices with the next release.

ikarus23 commented 10 months ago

next try

twisteroidambassador commented 10 months ago

Exporting to MCF looks correct, and reimporting the file is successful as well. Looks like the issue is fixed.

Happy new year!

ikarus23 commented 10 months ago

Thank you so much. Really nice work on finding the iterator issue. I did another version with more drastic changes on the issue that caused the complete crash. If you find the time you can test if it runs (I don't own an Android 4.4 device): https://tests.icaria.de/MifareClassicTool-4.1.0-test6.apk

The export fix is pushed and will be part of the next release. Happy new year to you to!

twisteroidambassador commented 10 months ago

Just installed test6, it launches normally and seem to work fine. Cheers!