Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

java.lang.IndexOutOfBoundsException: Index 12 out of bounds for length 12 #1739

Closed
kiber-io opened this issue Dec 1, 2022 · 9 comments
Closed

Comments

@kiber-io
Copy link

kiber-io commented Dec 1, 2022

When trying to switch the MainActivity class to smali view, an error occurs :(

  • Jadx version: 1.4.5
  • Java version: 19.0.1
  • Java VM: Oracle Corporation OpenJDK 64-Bit Server VM
  • Platform: Windows 10 (10.0 amd64)
  • Max heap size: 5672 MB
  • Program args: -Xms128M -XX:MaxRAMPercentage=70.0 -XX:+UseG1GC -Dawt.useSystemAAFontSettings=lcd -Dswing.aatext=true -Djava.util.Arrays.useLegacyMergeSort=true

APK: https://disk.yandex.ru/d/n6yXdRpi9pE3xg

java.lang.IndexOutOfBoundsException: Index 12 out of bounds for length 12
	at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:100)
	at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:106)
	at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:302)
	at java.base/java.util.Objects.checkIndex(Objects.java:385)
	at java.base/java.util.ArrayList.get(ArrayList.java:427)
	at jadx.gui.device.debugger.smali.SmaliMethodNode.setParamReg(SmaliMethodNode.java:95)
	at jadx.gui.device.debugger.smali.Smali.writeParamInfo(Smali.java:492)
	at jadx.gui.device.debugger.smali.Smali.formatMthParamInfo(Smali.java:475)
	at jadx.gui.device.debugger.smali.Smali.writeMethod(Smali.java:287)
	at jadx.gui.device.debugger.smali.Smali.lambda$writeClass$1(Smali.java:222)
	at jadx.plugins.input.dex.sections.DexClassData.readMethods(DexClassData.java:170)
	at jadx.plugins.input.dex.sections.DexClassData.visitMethods(DexClassData.java:145)
	at jadx.plugins.input.dex.sections.DexClassData.visitFieldsAndMethods(DexClassData.java:112)
	at jadx.gui.device.debugger.smali.Smali.writeClass(Smali.java:206)
	at jadx.gui.device.debugger.smali.Smali.writeClass(Smali.java:230)
	at jadx.gui.device.debugger.smali.Smali.disassemble(Smali.java:89)
	at jadx.gui.device.debugger.DbgUtils.lambda$getSmali$0(DbgUtils.java:32)
	at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1228)
	at jadx.gui.device.debugger.DbgUtils.getSmali(DbgUtils.java:31)
	at jadx.gui.device.debugger.DbgUtils.getSmaliCode(DbgUtils.java:36)
	at jadx.gui.ui.codearea.SmaliArea$DebugModel.load(SmaliArea.java:222)
	at jadx.gui.ui.codearea.SmaliArea.load(SmaliArea.java:94)
	at jadx.gui.ui.codearea.CodePanel.load(CodePanel.java:117)
	at jadx.gui.ui.codearea.ClassCodeContentPanel.lambda$buildTabbedPane$0(ClassCodeContentPanel.java:89)
	at java.desktop/javax.swing.JTabbedPane.fireStateChanged(JTabbedPane.java:446)
	at java.desktop/javax.swing.JTabbedPane$ModelListener.stateChanged(JTabbedPane.java:297)
	at java.desktop/javax.swing.DefaultSingleSelectionModel.fireStateChanged(DefaultSingleSelectionModel.java:148)
	at java.desktop/javax.swing.DefaultSingleSelectionModel.setSelectedIndex(DefaultSingleSelectionModel.java:79)
	at java.desktop/javax.swing.JTabbedPane.setSelectedIndexImpl(JTabbedPane.java:650)
	at java.desktop/javax.swing.JTabbedPane.setSelectedIndex(JTabbedPane.java:625)
	at java.desktop/javax.swing.plaf.basic.BasicTabbedPaneUI$Handler.mousePressed(BasicTabbedPaneUI.java:4140)
	at com.formdev.flatlaf.ui.FlatTabbedPaneUI$Handler.mousePressed(FlatTabbedPaneUI.java:2593)
	at java.desktop/java.awt.AWTEventMulticaster.mousePressed(AWTEventMulticaster.java:287)
	at java.desktop/java.awt.Component.processMouseEvent(Component.java:6617)
	at java.desktop/javax.swing.JComponent.processMouseEvent(JComponent.java:3398)
	at java.desktop/java.awt.Component.processEvent(Component.java:6385)
	at java.desktop/java.awt.Container.processEvent(Container.java:2266)
	at java.desktop/java.awt.Component.dispatchEventImpl(Component.java:4995)
	at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2324)
	at java.desktop/java.awt.Component.dispatchEvent(Component.java:4827)
	at java.desktop/java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4948)
	at java.desktop/java.awt.LightweightDispatcher.processMouseEvent(Container.java:4572)
	at java.desktop/java.awt.LightweightDispatcher.dispatchEvent(Container.java:4516)
	at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2310)
	at java.desktop/java.awt.Window.dispatchEventImpl(Window.java:2780)
	at java.desktop/java.awt.Component.dispatchEvent(Component.java:4827)
	at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:775)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:720)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:714)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:97)
	at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:747)
	at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:745)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86)
	at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:744)
	at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

@jmlitfi
Copy link
Contributor

jmlitfi commented Dec 9, 2022

I am able to reproduce this issue. The **void onItemClick(AdapterView<?> arg0, View arg1, int arg2, long arg3) ** method is causing it to crash on the MainActivity. it also crashes on the ColorListActivity class caused by the same onItemClick method within that class.

Crash is caused because the codeReader.getRegistersCount() function is returning 12 to set the size of the regList on SmaliMethodNode.java. OutOfBounds occurs when the Smali.formatMthParamInfo function is attempting to set at least 13 registries. It crashes when in the for loop iterating over the types List. I need to do more testing to figure out why the incorrect number of registries is being returned.

@jmlitfi
Copy link
Contributor

jmlitfi commented Dec 11, 2022

About to submit a pull request with a fix. See below for my analysis on what caused this issue.

Bug Cause
Within the Smali.formatMthParamInfo function when writing the dbgInfo variables a check happens where if var.getOffsetStart() == -1 it skips that variable (likely assuming its not a parameter). However, in the provided application the AdapterView<?> parameter variable was being sent as a DBG_START_LOCAL_EXTENDED and a null name was being passed in the parameter array. JADX expects that all arguments are sent in the parameter array which will get an offset of -1. Anything not sent in the parameter name array gets the true offset meaning the check for -1 will be false meaning JadX will skip that variable and not increment the RegNum or ParamCount in Smali.formatMthParamInfo. The outOfBounds occurred because later in the formatMthParamInfo function it will take the remaining parameters and iterate over them. However, because JADX didn't increment the param count or regNum it is reading the last variable twice which in this cases is a Long meaning it will read two Longs (2 registries each) instead of a Long and Object (1+2 registries) resulting in an OutOfBounds

Fix
My fix replaces var.getOffsetStart() == -1 with var.getRegNum() == regNum within the Smali.formatMthParamInfo function. This will make sure the returned debug variables are part of the parameters by comparing the var.regNum with the parameter regNum as determined using the getParamRegStart(). The current way of checking for parameters will fail anytime an argument is sent outside of the parameters array.

jmlitfi pushed a commit to jmlitfi/jadx that referenced this issue Dec 11, 2022
@skylot
Copy link
Owner

skylot commented Dec 11, 2022

@jmlitfi thanks for the fix, it looks good to me 👍
But this method is overcomplicated, so I want to add a unit test for this case (similar to DbgSmaliTest). Such test will allow to debug and understand this method logic and maybe rewrite it to make easier to understand. This will take some time, so please wait for updates.

@jmlitfi
Copy link
Contributor

jmlitfi commented Dec 11, 2022

I was thinking about my fix last night and it only works if "for (ILocalVar var : dbgInfo.getLocalVars())" reads the registries in order. As far as I can tell it should always be in order but if you think getLocalVars() could return out of order it might make sense to save the start of the parameter registries and do a greater than instead of doing the == as I did. As long as we have a reliable way of checking if a registry is part of the parameters or local it should work.

Agreed that a unit test makes sense. To assist in readability I think it would also help to add comments on which function handles which part of the bytecode format as it took a bit of time to narrow down which functions parse what.

skylot pushed a commit that referenced this issue Dec 13, 2022
Co-authored-by: jmlitfi <jeffmlitfi@gmail.com>
@skylot
Copy link
Owner

skylot commented Dec 13, 2022

@jmlitfi I merged your PR and pushed my commit (0fafcfa) with test and method rewrite to simplify code. Hope I don't introduce new issues, but if possible please check my changes 🙂

@skylot skylot closed this as completed Dec 13, 2022
@jmlitfi
Copy link
Contributor

jmlitfi commented Dec 14, 2022

@skylot I am a bit concerned that your changes doesn't fix the actual problem. I dont think that using the var.getStartOffset() <= 0 is a reliable way of checking if something is a parameter. I could be wrong but I think you need to calculate the start of parameter registries (line.smaliMthNode.getParamRegStart()) then do a check if the variable reg number (var.getRegNum()) is greater than or equal to the start of the parameter registries.

My understanding of the dex format from reading over https://source.android.com/docs/core/runtime/dex-format#debug-info-item is that parameters should be passed in the "parameter_names" array which is what JadX checked for originally as anything in the parameter_names array will get a -1 for getStartOffset(). I dont fully understand what is causing it but looking at the apk shared by kiber-io the first value of the parameter_names array is null (which JadX will skip).

Directly after the "parameter_name" array the dex format will send the DBG variables with the first DBG variable having a getStartOffset() of 0. In the provided APK the first DBG variable is DBG_START_LOCAL_EXTENDED which comparing the registry number with the calculated start registry for the parameters I can tell it is the first parameter.

jmlitfi pushed a commit to jmlitfi/jadx that referenced this issue Dec 14, 2022
@jmlitfi
Copy link
Contributor

jmlitfi commented Dec 14, 2022

See the below image for my debugging messages. The DebugInfoParser class handles parsing of the debug_info_item which populates the variables.

I pushed my log statements to the https://github.com/jmlitfi/jadx/tree/issue_1739 branch to help with tracing the code flow.

image

@skylot
Copy link
Owner

skylot commented Dec 25, 2022

@jmlitfi I understand your confusion here, because in debug info first parameter (arg0) declared as a local variable. But as soon as we are trying to recreate original smali here (which is a raw dex representation), we just print that debug info as is.
In new commit I replaced offset check with an additional method and also fixed several issues, so now generated code is same as original smali:

    .registers 10
    .param p2, "arg1"    # Landroid/view/View;
    .param p3, "arg2"    # I
    .param p4, "arg3"    # J
    .line 69
    .local p1, "arg0":Landroid/widget/AdapterView;, "Landroid/widget/AdapterView<*>;"

@jmlitfi
Copy link
Contributor

jmlitfi commented Dec 25, 2022

@skylot Thanks for your commit. I have not tested but reading over your code I think it is a good way to handle this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants