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

Fix tracer return value parse error #483

Closed
wants to merge 4 commits into from

Conversation

commonuserlol
Copy link

When a method definition does not include an null value (example: MyClass getMyClass();) but something went wrong (or null check was stripped by il2cpp compiler) it can return NULL so Error: abort was called will be thrown (I think because NativeFunction expected pointer return type but got null)
One more note, "real" IL2CPP code checks if the value is null (at least ghidra decompiler shows that), so nothing bad should happen

Before:

il2cpp: 
0x00d42bc0 ┌─MyClass::.ctor(this =  (MyClass))
0x00d42bc0 └─MyClass::.ctor

Error: abort was called
    at callback (/home/commonuserlol/index.js:1316)

After:

il2cpp: 
0x00d42bc0 ┌─MyClass::.ctor(this =  (MyClass))
0x00d42bc0 └─MyClass::.ctor

il2cpp: 
0x00d41af4 ┌─MyClass::OnEnable(this = SOME_VIEW(Clone) (MyClass))
0x00d41e48 │ ┌─MyClass::GetNextReward(this = SOME_VIEW(Clone) (MyClass))
0x00d41e48 │ └─MyClass::GetNextReward = null
0x00d422c8 │ ┌─MyClass::ParseReward(this = SOME_VIEW(Clone) (MyClass), reward = null)
0x00d421f0 │ │ ┌─MyClass::OnOkButton(this = null)
0x00d421f0 │ │ └─MyClass::OnOkButton
0x00d422c8 │ └─MyClass::ParseReward
0x00d41af4 └─MyClass::OnEnable

@vfsfitvnm
Copy link
Owner

Thanks, I'm fine with that. But perhaps it would be more meaningful if we reported it, even a simple message is fine something like:

0x00d41e48 │ ┌─MyClass::GetNextReward(this = SOME_VIEW(Clone) (MyClass))
0x00d41e48 │ └─MyClass::GetNextReward = null [native IL2CPP excpetion occurred]

Bonus points if you make it red!

What do you think?

@commonuserlol
Copy link
Author

commonuserlol commented Feb 7, 2024

I haven’t seen something similar here, how would you implement a value check??

  1. ret.equals(NULL) ? "err" : (ret == undefined ? "" : "formatted ret")
  2. if (ret.equals(NULL)) ...; else if (...); else ...

@commonuserlol
Copy link
Author

commonuserlol commented Feb 7, 2024

Okay, now it looks like

0x00d41e48 │ ┌─MyClass::GetNextReward(this = SOME_VIEW(Clone) (MyClass))
0x00d41e48 │ └─MyClass::GetNextReward = null [native IL2CPP excpetion occurred]

Also colored like error from console.ts/raise
image

I did with 1st variant from my previous message (not commited yet)

                    const result = returnValue == undefined ? ""
                        : (returnValue.equals(NULL) ?
                        " = \x1b[0m\x1b[38;5;9mnull [native IL2CPP excpetion occurred]\x1b[0m"
                        : ` = \x1b[36m${fromFridaValue(returnValue, method.returnType)}\x1b[0m`);

If you prefer other style let me know

@commonuserlol
Copy link
Author

Correct impl is

                    const result = returnValue == undefined ? "" :
                        returnValue instanceof NativePointer ?
                            returnValue.equals(NULL)
                            ? " = \x1b[0m\x1b[38;5;9mnull [native IL2CPP excpetion occurred]\x1b[0m"
                            : ` = \x1b[36m${fromFridaValue(returnValue, method.returnType)}\x1b[0m`
                        : ` = \x1b[36m${fromFridaValue(returnValue, method.returnType)}\x1b[0m`;

now tracer works (in prev piece of code it died when returnValue was non-native pointer)
Still waiting for answer about codestyle

@vfsfitvnm
Copy link
Owner

Hmm, this is how I would do it:

                let returnValue;
                let isError = false;
                try {
                    returnValue = method.nativeFunction(...args);
                } catch (_) {
                    isError =  true;
                }

                if ((this as InvocationContext).threadId == threadId) {
                    // prettier-ignore
                    state.buffer.push(`\x1b[2m0x${paddedVirtualAddress}\x1b[0m ${`│ `.repeat(--state.depth)}└─\x1b[33m${method.class.type.name}::\x1b[1m${method.name}\x1b[0m\x1b[0m${returnValue == undefined ? "" : ` = \x1b[36m${fromFridaValue(returnValue, method.returnType)}`}\x1b[0m${isError ? " \x1b[38;5;9m[native IL2CPP excpetion occurred]\x1b[0m" : ""}`);
                    state.flush();
                }

                return isError ? NULL : returnValue;

However, I'm wondering what happens in case Frida expect us to return a number (let's say an exception occurs within System.In32 Foo();) - I believe we can't always return NULL (a NativePointer), can we?

@commonuserlol
Copy link
Author

commonuserlol commented Feb 8, 2024

Il2Cpp compiled code already have checks for NativePointer but for number it can be UB (undefined behavior) as c++ have it. Frida can interpret it as 0 afaik. Probably easiest way to check is replacing some method with Int32 return type (NativeFunction provides exceptions option which stops generating JS exception and app should handle it by itself) and throw some exception in it. But lemme test this tomorrow (here is 0:45)

@commonuserlol
Copy link
Author

commonuserlol commented Feb 8, 2024

Actually we can shift responsibility to the app (it will handle NULL), what do think about this?
upd: nvm it crashes with this

@Flechaa
Copy link
Contributor

Flechaa commented Feb 8, 2024

Hmm, this is how I would do it:

                let returnValue;
                let isError = false;
                try {
                    returnValue = method.nativeFunction(...args);
                } catch (_) {
                    isError =  true;
                }

                if ((this as InvocationContext).threadId == threadId) {
                    // prettier-ignore
                    state.buffer.push(`\x1b[2m0x${paddedVirtualAddress}\x1b[0m ${`│ `.repeat(--state.depth)}└─\x1b[33m${method.class.type.name}::\x1b[1m${method.name}\x1b[0m\x1b[0m${returnValue == undefined ? "" : ` = \x1b[36m${fromFridaValue(returnValue, method.returnType)}`}\x1b[0m${isError ? " \x1b[38;5;9m[native IL2CPP excpetion occurred]\x1b[0m" : ""}`);
                    state.flush();
                }

                return isError ? NULL : returnValue;

However, I'm wondering what happens in case Frida expect us to return a number (let's say an exception occurs within System.In32 Foo();) - I believe we can't always return NULL (a NativePointer), can we?

By the way, there's a typo here, it should be exception not excpetion.

@commonuserlol
Copy link
Author

I found method which already throws error (but target still works)

il2cpp: System.NullReferenceException: Object reference not set to an instance of an object.
  at User.GetBalance (System.String a) [0x00000] in <00000000000000000000000000000000>:0
il2cpp:
0x00ee1ad4 ┌─User::GetBalance(this = name, a = "remove_ads")
0x00ee1ad4 └─User::GetBalance = null [native IL2CPP excpetion occurred]

Error: expected an integer // due returning NULL

after commit:

il2cpp:
0x00ee1ad4 ┌─User::GetBalance(this = name, a = "remove_ads")
0x00ee1ad4 └─User::GetBalance = 0 [native IL2CPP exception occurred]
// No Error: expected an integer

For other types it should work

@commonuserlol
Copy link
Author

Note: void is fine with both NULL and 0 (not sure it's your code or frida default behavior), tested with:

    Backend.method("HandleResponse").implementation = function (req) {
        this.method<void>("HandleResponse").invoke(req);
        // let's imagine that an exception was caught (abort was called), we returning null
        console.log("test: return NULL for System.Void HandleResponse(HttpRequest request);");
        console.log(`isPrimitive for original ret (void): ${this.method<void>("HandleResponse").returnType.isPrimitive}`)
        return NULL;
    }

output:

test: return NULL for System.Void HandleResponse(HttpRequest request);
isPrimitive for original ret (void): false
// No error, 0 is ok too

@vfsfitvnm
Copy link
Owner

const cm = ((globalThis as any).cm = new CModule(`int lol(void) { return 1; }`));
Interceptor.replace(cm.lol, new NativeCallback(() => NULL as any, "int", []));
console.log(new NativeFunction(cm.lol, "int", [])());

I get Error: expected an integer

@commonuserlol
Copy link
Author

Look, i pushed one more commit which does method.returnValue.isPrimitive ? 0 : NULL (int/System.Int32 is primitive)

@commonuserlol
Copy link
Author

ping

@AsukaWhite
Copy link

Sorry for bothering. looks like you are familiar with this module. would you mind help my question in issue?
#506
it tortures me, thanks in advance.

@commonuserlol
Copy link
Author

@vfsfitvnm please review this pr

@Vladik01-11
Copy link

@vfsfitvnm ping

@fakekey
Copy link

fakekey commented Aug 10, 2024

@commonuserlol Hi, I think I'm having the same issue here. I've tried building your fork and there were 4 errors before, but now there are only 2 left. Could you please take a look? I'd really appreciate it.

- Error: abort was called
-   at invokeRaw (structs/method.ts:234)
-   at toString (structs/object.ts:58)  
-   at concat (native)
-   at <anonymous> (tracer.ts:297)      
-   at map (native)
-   at callback (tracer.ts:297)
- Error: access violation accessing 0x10  
-   at get length (structs/string.ts:26)
-   at get content (structs/string.ts:5)
-   at toString (structs/object.ts:58)  
-   at concat (native)
-   at <anonymous> (tracer.ts:297)      
-   at map (native)
-   at callback (tracer.ts:297)

@commonuserlol
Copy link
Author

@fakekey Hi, I haven't these error when tested, can you tell game name (or send link)

@Vladik01-11
Copy link

@fakekey Hi, I haven't these error when tested, can you tell game name (or send link)

I can suggest you take a look at games.flexus.trainminer, in fact there are the same mistakes that @fakekey said

@commonuserlol
Copy link
Author

I see. I think we shouldn't flood there. Please create issue on my repo or you can DM me at discord (same name as on github) to track it. I'll look into tomorrow

Before we used Interceptor.replace which called `NativeFunction`.
For reasons that are not very clear to me, it could throw "abort was called", which made us return a null pointer and possibly crash the program.
Now we use Interceptor.attach and thus do not interrupt the original execution.

For example:

previous implementation
il2cpp:
0x010d4c94 ┌─UserTargeting.HamsterOfferwall::get_ActiveReward(this = HamsterOfferwall (UserTargeting.HamsterOfferwall))
0x010d4c94 └─UserTargeting.HamsterOfferwall::get_ActiveReward = null [native IL2CPP exception occurred] // <- null pointer returned

after that the app crashed.

new implementation
il2cpp:
0x010d4c94 ┌─UserTargeting.HamsterOfferwall::get_ActiveReward(this = HamsterOfferwall (UserTargeting.HamsterOfferwall))
0x010d4c94 └─UserTargeting.HamsterOfferwall::get_ActiveReward = ""

and execution continues as desired, yay!

One more important improvement, as @fakekey and @Vladik01-11 noticed, some apps have issues with getting the parameter value (again, for reasons I don't understand).
Now we check if it is possible to get the parameter value, instead of spamming the console with errors, if it is impossible to get the parameter value (as I noticed, this applies mostly to Il2Cpp.Object) you will see the red text "null (failed to parse parameter value)".
@commonuserlol
Copy link
Author

Hi @fakekey, @Vladik01-11 - reproduced, pushed and tested on games.flexus.trainminer. Should be fine now

@commonuserlol
Copy link
Author

Confirmed by fakekey (via discord)

@commonuserlol
Copy link
Author

Reverted due return value parsed incorrectly.

@commonuserlol commonuserlol closed this by deleting the head repository Aug 17, 2024
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

Successfully merging this pull request may close these issues.

6 participants