frida / frida-objc-bridge

Objective-C runtime interop from Frida
49 stars 21 forks source link

requirement for exporting function 'parseSignature' or 'parseType' #19

Closed gebing closed 4 years ago

gebing commented 5 years ago

@oleavr Is it possible to export function 'parseSignature' or 'parseType', so that we can use it to parse method types's signature and use 'toNative' or ‘fromNative’ to convert values between javascript and native pointer?

oleavr commented 4 years ago

Sounds like a good idea to me. But first it would be good to port this module to TypeScript (example module here).

gebing commented 4 years ago

That will be a lot of work, do you need some help?

oleavr commented 4 years ago

Yes help would be awesome 😍🙌

gebing commented 4 years ago

This month i am a little bit busy, but i can make some help later.

gebing commented 4 years ago

@oleavr How can i start porting to TypeScript? Should i just fork frida-objc-bridge and code like the example module?

oleavr commented 4 years ago

@gebing Yes that sounds like the way to go. Style should follow this guide.

gebing commented 4 years ago

@oleavr

I started the work and first tried to porting api.js to Typescript. Do you have time to see if it is ok? If it's ok, i will continue to other files.

The api.ts is on https://github.com/gebing/frida-objc-bridge/blob/master/lib/api.ts

gebing commented 4 years ago

By the way, i have some question about the code of api.js, what does this code for?

line 123-125:
                    signature.call(temporaryApi, exp.address);
                    if (isObjCApi)
                        signature.call(temporaryApi, exp.address);

line 128-129:
                    if (isObjCApi)
                        temporaryApi[name] = temporaryApi[name];
oleavr commented 4 years ago

Sorry for the delay, got distracted.

Looks good except for checks like if (!temporaryApi.objc_msgSend_stret), which should be using this style: if (temporaryApi.objc_msgSend_stret === undefined). (Mentioned in the style guide.)

By the way, i have some question about the code of api.js, what does this code for?

Wow, that looks like some code that was left over from a refactoring. It doesn't do anything useful and should be removed.

gebing commented 4 years ago

OK, i will fix it and make more porting.

There is an error when i tried to compile the typescript, and i think the declaration of enumerateExportsSync should be added to frida-gum's index.d.ts:

lib/api.ts:134:10 - error TS2339: Property 'enumerateExportsSync' does not exist on type 'typeof Module'.
oleavr commented 4 years ago

Awesome!

i think the declaration of enumerateExportsSync should be added to frida-gum's index.d.ts

No, that's deprecated API that shouldn't be used. It has been replaced by enumerateExports() without callbacks.

gebing commented 4 years ago

@oleavr According to contributing guide, lines should not exceed 80 characters, but 80 characters will cause many wrapped lines because of typescript's type declaration and assignment, and actually many of the original javascript code are exceed 80 characters.

I suggest expand line characters to 120 characters.

oleavr commented 4 years ago

@gebing The 80 characters rule isn't generic, it applies to frida-gum specifically. But yes I think we can even say 140 characters, like the code in frida-core.

gebing commented 4 years ago

@oleavr Please fix the declare error of frida-gum's index.d.ts. I will get compile error if not fixed. Please refer frida-gum's issue #358 which i submitted before.

gebing commented 4 years ago

@oleavr There is a error in index.js of line 831:

827:        function flatProtocolMethods(result, protocol) {
828:            if (protocol.methods !== undefined) {
829:                Object.assign(result, protocol.methods);
830:            }
831:            if (protocol.protocol !== undefined) {
832:                flatProtocolMethods(result, protocol.protocol);
833:            }
834:            return result;
835:        }

Where the protocol come from, i didn't see any declaration of it and it also should not be protocols because of the signature of flatProtocolMethods do not accept protocols.

oleavr commented 4 years ago

@gebing

Please fix the declare error of frida-gum's index.d.ts. I will get compile error if not fixed. Please refer frida-gum's issue #358 which i submitted before.

Will do on the next update. (To be submitted right after the release has been pushed.)

There is a error in index.js of line 831: … Where the protocol come from …

It's passed to it through its second argument. stealTypesFromProtocols calls it here:

                .map(protocolName => flatProtocolMethods({}, klass.$protocols[protocolName]))
gebing commented 4 years ago

@oleavr The second argument of stealTypesFromProtocols should be type of Object.Protocol, and Object.Protocol do not have the property of protocol.

According the code of Object.Protocol, it only has the properties of handle, name, protocols, properties and methods, so i wonder where does the protocol come from?

gebing commented 4 years ago

Typescript will check the property name by the type declaration strictly, so no type definition for protocol will compile error.

gebing commented 4 years ago

@oleavr Another fix requirement for frida-gum's index.d.ts:

    interface ProxySpec<D extends ProxyData = ProxyData, T = ObjC.Object, S = ObjC.Object> {
        /**
         * Protocols this proxy class conforms to.
         */
        protocols?: Protocol[];

        /**
         * Methods to implement.
         */
        methods?: {
            [name: string]: UserMethodImplementation<D, T, S> | MethodSpec<UserMethodImplementation<D, T, S>>;
        };

        /**
         * Callbacks for getting notified about events.
         */
        events?: ProxyEventCallbacks<D, T, S>;
    }

ProxySpec should has an optional property of name, which used in function registerProxy as below, and will cause typescript compile error:

        const ProxyClass = registerClass({
            name: properties.name,
            super: classRegistry.NSProxy,
            protocols: protocols,
            methods: proxyMethods
        });
gebing commented 4 years ago

@oleavr I will wait for your new release of frida-objc-bridge to resume my porting for typescript.

gebing commented 4 years ago

@oleavr I seems that the frida-gum's index.d.ts is still not updated, when it can be submitted?

gebing commented 4 years ago

@oleavr Another fix requirement for frida-gum's index.d.ts:

    class ObjectMethod implements ObjectWrapper {
        handle: NativePointer;

        /**
         * Objective-C selector. Use `ObjC.selectorAsString()` to convert it to a string.
         */
        selector: NativePointer;

        /**
         * Current implementation.
         *
         * You may replace it by assigning to this property. See `ObjC.implement()` for details.
         */
        implementation: NativePointer;

        /**
         * Return type name.
         */
        returnType: string;

        /**
         * Argument type names.
         */
        argumentTypes: string[];

        /**
         * Signature.
         */
        types: string;
    }

Declaration of ObjectMethod should change to interface ObjectMethod extends ObjectWrapper, Function, because in index.js's line 881, there exist a call for ObjectMethod's own method as below:

881:       implemented = (respondsToSelector !== null && respondsToSelector.call(self, selector(methodName)));

Typescript compiler will be error for TS2339: Property 'call' does not exist on type 'ObjectMethod'.

oleavr commented 4 years ago

Thanks! Could you fork the DefinitelyTyped repo and commit the changes? Just so I don't forget any. I will submit the PR to them as soon as Frida 12.8.0 is out.

oleavr commented 4 years ago

(So if you make a branch fixing the issues you've found, I can just merge that and add the new API additions on top of it. It's already a lot of things to remember so I'm afraid of forgetting something 😅)

gebing commented 4 years ago

OK.

gebing commented 4 years ago

@oleavr It is too complicate to submit a pull request for DefinitelyTyped repo, so i clone the repo at https://github.com/gebing/DefinitelyTyped, and commit my modification. If i have any other modification requests, i can modify the types/frida-gum/index.d.ts and commit the changes, so that you can merge into frida-gum's release.

oleavr commented 4 years ago

@gebing Thank you, that's exactly what I meant. I will merge those changes now and submit a PR upstream.

oleavr commented 4 years ago

TypeScript bindings PR submitted: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/41109

gebing commented 4 years ago

@oleavr The type of ObjectMethod's property implementation should be NativeFunction, but now is NativePointer:

    interface ObjectMethod extends ObjectWrapper, AnyFunction {
        handle: NativePointer;

        /**
         * Objective-C selector. Use `ObjC.selectorAsString()` to convert it to a string.
         */
        selector: NativePointer;

        /**
         * Current implementation.
         *
         * You may replace it by assigning to this property. See `ObjC.implement()` for details.
         */
        implementation: NativePointer;

        /**
         * Return type name.
         */
        returnType: string;

        /**
         * Argument type names.
         */
        argumentTypes: string[];

        /**
         * Signature.
         */
        types: string;

        /**
         * Makes a new method wrapper with custom NativeFunction options.
         *
         * Useful for e.g. setting `traps: "all"` to perform execution tracing
         * in conjunction with Stalker.
         */
        clone: (options: NativeFunctionOptions) => ObjectMethod;
    }
oleavr commented 4 years ago

@gebing https://github.com/gebing/DefinitelyTyped/commit/f1689d186b9cbdef060a22a8cb34c924389df2ee#r36477047