vfsfitvnm / frida-il2cpp-bridge

A Frida module to dump, trace or hijack any Il2Cpp application at runtime, without needing the global-metadata.dat file.
https://github.com/vfsfitvnm/frida-il2cpp-bridge/wiki
MIT License
946 stars 194 forks source link

Add wrapper for `System.Collections.Generic.Dictionary` #345

Closed commonuserlol closed 8 months ago

commonuserlol commented 1 year ago

Hey, this little change should save you a lot of extra code. You can test it using this:

import "frida-il2cpp-bridge";

function test() {
    const myMap = new Map<number, Il2Cpp.String>([ [1, Il2Cpp.string("a")], [2, Il2Cpp.string("b")] ]);
    const dictionary = Il2Cpp.dictionary(Il2Cpp.corlib.class("System.Int32"), Il2Cpp.corlib.class("System.String"), myMap);

    dictionary.set(2, Il2Cpp.string("z")); //should show warn
    dictionary.set(3, Il2Cpp.string("c"));

    console.log(dictionary.get(2)); //should be "z"
    console.log(dictionary.containsKey(4)); //should be false

    dictionary.set(4, Il2Cpp.string("d"));

    console.log(dictionary.containsValue(Il2Cpp.string("d"))); //should be true
    console.log(dictionary.length); //should be 4
    console.log(dictionary.find(3)); //should be 2
    console.log(dictionary.keys); //should be 1, 2, 3, 4
    console.log(dictionary.values); //should be "a", "z", "c", "d"
    console.log(dictionary.toString()); //should be {1: "a", ...}

    for (const [key, value] of dictionary.entries) {
        console.log(`${key} - ${value}`); //should be 1 - "a", ...
    }

    dictionary.clear();

    console.log(dictionary.length); //should be 0
    console.log(dictionary.get(1)); //should show err
}
//@ts-ignore
globalThis.test = test;

after app loaded you can call test in frida repl

I was able to test only on android, but i think it works on other platforms. Note: A ctor with a size exists like in array, but is essentially useless because this class has a dynamic length. Also find returns -1 if it doesn't find the key. I tried to follow your coding style so there shouldn't be much difference

vfsfitvnm commented 1 year ago

Hey, thanks for taking your time to make this PR. The code looks fine (a minor thing, wrappers should be probably located in a directory named wrappers).

However. my original idea was to stay as minimal as possible - that's why I didn't implement any wrapper yet (Il2Cpp.String and Il2Cpp.Array are real (?) IL2CPP structs).

I'm not against the addition of wrapper classes, but we should be very careful with the design.

1) We must make sure the used methods are available on every Unity version AND they were not stripped during compilation. This is unlikely to happen for a such frequently used class - but such scenario should be covered to some extent. I don't really want a wobbly API that might not work (again, System.Collections.Generic.Dictionary is unlikely to be affected, but still...); 2) How we determine whether a class is worth being wrapped? We risk to have a maintainability problem; 3) How to choose names? There's no Il2CppDictionary in the IL2CPP source code - is Il2Cpp.Dictionary fine? 4) Should Il2Cpp.Method::invoke and Il2Cpp.Field::value return a Il2Cpp.Dictionary instead of a Il2Cpp.Object? If so, would such thing add a considerable runtime penalty? Also, it would make core depend on wrappers - which is a domain violation to me. But the user would expect invoke to return Il2Cpp.Dictionary - as it can already handle Il2Cpp.String and Il2Cpp.Array. 5) Should Il2Cpp.String and Il2Cpp.Array be demoted to wrappers as well? They have their IL2CPP struct, but they are just built on top of Il2CppObject.

These are my doubts.

That's why I was considering to programmatically create TS wrappers (https://github.com/vfsfitvnm/frida-il2cpp-bridge/issues/322#issuecomment-1613549080)

commonuserlol commented 1 year ago

I think that there are not so many wrappers. Basically it's "vital" (no, for reverse engineer) inner classes like List<T> or Dictionary<TKey, TValue>. About a Il2Cpp.String and Il2Cpp.Array, most likely they should remain in the same form as they are now. If you are against the idea of wrappers, then you should do something like "ready-made wrappers for internal classes" in the wiki or smth. I would say that it just simplifies life and only. The choice to use wrappers or not is still up to you...

commonuserlol commented 1 year ago

so? I don't wanna sound annoying, but have you made up your mind?

vfsfitvnm commented 1 year ago

Not yet as I'm currently concentrated on another project of mine; this is a delicate topic and requires time to reason about - I cannot context switch right now or make hasty decisions. Time is scarce, unfortunately!

commonuserlol commented 10 months ago

Uh so? Yes, I know it doesn’t sound very good and that your question is about minimalism, but just merge or decline

commonuserlol commented 8 months ago

Sore i have no more time to wait