cherriesandmochi / gdmaim

GDMaim is a GDScript obfuscation plugin for the Godot Engine.
MIT License
145 stars 13 forks source link

BUG: variable with variant that have engine singleton assigned will obfuscate #5

Closed normano closed 7 months ago

normano commented 8 months ago

Had a bug right under our noses. If you look at haptics addon in the experiment repo you'll see that the function called on the singleton is obfuscated, but should not be. The variable itself is declared variant, so anything referenced on it should be left alone since there's not much information on the variant to do any obfuscation.

This is easily mitigated by using the obfuscate false on these kinds of scripts which are typically addons.

The one that is not mitigated by ##obfuscate false is when singleton.init() is called from an addon. I demonstrate this in the window.gd code.

cherriesandmochi commented 8 months ago

Not sure if I'm misunderstanding something, but singletons and consequentially calls to it, are supposed to get obfuscated. ##OBFUSCATE false only works on declarations being made, which is by design as completely stopping snippets from getting obfuscated, without resolving any global symbols, will usually break any code. It might make sense to rename the hint to something like ##OBFUSCATE_SYMBOL, to make it clearer what it actually does.

normano commented 8 months ago

This engine singleton in this case is defined and compiled in a plugin from a different language. For example: https://github.com/GhostWalker562/Godot-Haptic-Feedback-Module-IOS/blob/master/haptic/register_types.cpp No information is known about this singleton until runtime.

Also singleton.init() in this case is a regular function. I personally think people should use something more conttrast like setup or something, but this kind of name is also used by official godot plugins. I expected that obfuscate false would've stopped obfuscation or any resolution here, but I get what you're saying.

cherriesandmochi commented 8 months ago

I did not think about external plugins at all. Okay, that's going to be a bit tricky.

The only automated way I can think of is to somehow try to load the dynamic libraries during export and then using ClassDB to fetch all methods and properties. But alas, I'm not quite sure if that's possible. Alternatively, it might actually be possible to launch a fake instance of Godot using all target dynamic libraries, which could then report back with all methods and properties gathered.

Not sure yet, how feasible both of those approaches are, the first one is probably not possible in the first place. For now, the 'best' workaround is manually stopping all the plugin methods and properties from being obfuscated I guess. Or at least those you want to access from GDScript. Something like:

##OBFUSCATE false
var plugin_property
func init(): pass
func plugin_method(): pass
func another_plugin_method(): pass
##OBFUSCATE true

Ah okay, well if init is a commonly used plugin method, I could make an exception and add it to the built-ins, at least until I have figured out how to deal with external plugins automatically.

cherriesandmochi commented 8 months ago

I expected that obfuscate false would've stopped obfuscation or any resolution here, but I get what you're saying.

I agree, it would make sense for ##OBFUSCATE false to stop any obfuscation. Then, a new hint called ##OBFUSCATE_SYMBOL could instead skip obfuscation of just declarations.

normano commented 8 months ago

I don't think loading external plugins is needed. The variable is declared variant so any methods or variables referenced on it should not be considered for obfuscation or suppose if the assignment to the variant cannot be resolved then the same result.

In the experiment repo at the bottom of world.gd:

class ExampleHaptic:
  var instance: Variant = null;
  func setup() -> void:
    if Engine.has_singleton("HapticEngine"):
      instance = Engine.get_singleton("HapticEngine");
      instance.init(); ##BUG Jan 29 - This is obfuscated for some   reason
      instance.some_call_from_haptic();
      instance.call();

The issue here is the init being renamed, but the others are not. I personally am fine with not renaming anything referenced on instance at all. If ##OBFUSCATE_SYMBOL false on "var instance" facilitated (obfuscate == false and type == variant) that then bug solved.

Like right now, I use obfuscate false on data classes that are used as serialization containers to keep the field names so they are 1-1 with the objects passed from external sources.

normano commented 8 months ago

Ok I see what's happening here, so TracerSubscriber defines init and that gets obfuscated to "OPmx" which is correctly used where TracerSubscriber calls init in the world.gd. Now in the ExampleHaptic class, TracerSubscriber's init token is reused that's why init here is rewritten to OPmx even though instance here is not a TracerSubscriber.

This looks like it would also be related to why haptics plugin instance.vibrate is renamed since vibrate is also a declared function in the script.

new_symbol = _global_symbols.get_symbol(token) is where it obtains the renamed symbol.

I can see why it is hard to do some reasoning though. There isn't really a way to see if the method token is tied to a type or maybe i don't understand what's going on enough. I was looking at ScriptData members and it doesn't appear to have that ability. Might be worth investigating using an AST parser in the future as maybe limits are reached here: https://github.com/Scony/godot-gdscript-toolkit

cherriesandmochi commented 8 months ago

Yes for sure, the parser I wrote is pretty much just thrown together to at least somewhat work. I do not really have experience with those. Using a 3rd party tool like the GDScript toolkit does seem very tempting.