CensoredUsername / unrpyc

A ren'py script decompiler
Other
859 stars 156 forks source link

Check for manipulated header fails sometimes #196

Closed madeddy closed 5 months ago

madeddy commented 7 months ago

For the current test of the rpyc2 header we use this code:

if not raw_contents.startswith(b"RENPY RPC2"):
    raise Exception("This isn't a normal rpyc file")

Problem is, it catches only changes in the 10 char length of the official header. If "someone" simply adds a few chars e.g. b"RENPY RPC2b" or b"RENPY RPC2 AoSC" the check runs strait over it and we get a total different error:

Error while decompiling /home/olli/.xlib/RPG/_test/AttackOnSurveyCorps-0-16-0-pc/game/dlc/hny_2024/event12.rpymc:
Traceback (most recent call last):
File "/home/olli/Code/Git/unrpyc/unrpyc.py", line 180, in worker
return decompile_rpyc(filename, args.clobber, args.dump, no_pyexpr=args.no_pyexpr,
File "/home/olli/Code/Git/unrpyc/unrpyc.py", line 109, in decompile_rpyc
ast = read_ast_from_file(in_file)
File "/home/olli/Code/Git/unrpyc/unrpyc.py", line 78, in read_ast_from_file
raw_contents = zlib.decompress(chunks[1])
KeyError: 1

This confuses a lot of people and because of the missing info most users don't get the idea to use the deobfucate feature which would deal with it.

If somehow possible this should be improved at some point, so the user can be correctly informed about it. As bonus, this could perhaps immediately send into the corresponding deobfuscate method.

CensoredUsername commented 7 months ago

Fair enough. The easiest way to check for this is to see if the position fields actually fit the expected pattern of 1, 2. If the header is wrong those fields likely contain garbage.

CensoredUsername commented 6 months ago

Added some warnings in that file around possible issues of loading old rpyc files, rpyc files that we can detect to be from ren'py 7 (doesn't catch all of them, but at least we can warn people that something is afoot).

Seems either way that the master decompiler is surprisingly good at still decompiling ren'py 7 files. So I'm really considering just getting the old-style argument support in there and at least having dev also be compatible with ren'py 7.

On another note, apparently we're also missing test.testast.If support.

madeddy commented 6 months ago

Added some warnings in that file around possible issues of loading old rpyc files

Looks good! I think this will go a long way for getting a first lead on things.

we're also missing test.testast.If support.

I guess this will not be the last thing that sneaked somehow around the Ren'Py docs or changelog. 😉

Btw. Whats now with missing pickletools, huh? Strange...! (I thought at first "how did i manage to breaks this now" with my mini-pull req?)

CensoredUsername commented 6 months ago

Btw. Whats now with missing pickletools, huh? Strange...! (I thought at first "how did i manage to breaks this now" with my mini-pull req?)

Apparently an issue in my previous un.rpyc overhauls. Instead of removing all renpy module loaders, I removed everything but the renpy module loaders. Which was fine, until it needed to import a previously not imported module from the stdlib.

madeddy commented 6 months ago

The wrong header falls still through with the current dev branch:

Error while decompiling /home/olli/.xlib/RPG/_test/OurBrightDays-0.1.3-pc/game/0_TEST_header_fake.rpyc:
Traceback (most recent call last):
File "/home/olli/Code/Git/unrpyc/unrpyc.py", line 247, in worker
return decompile_rpyc(filename, args.clobber, args.dump, no_pyexpr=args.no_pyexpr,
File "/home/olli/Code/Git/unrpyc/unrpyc.py", line 169, in decompile_rpyc
ast = read_ast_from_file(in_file)
File "/home/olli/Code/Git/unrpyc/unrpyc.py", line 122, in read_ast_from_file
contents = zlib.decompress(contents)
zlib.error: Error -3 while decompressing data: incorrect header check

I used a rpyc file with manipulated header where one char was changed. (RENPY RPC2 -> RENPY RPC3) The same effect can be reached, if one checks in unrpyc.py for a wrong header string.

The complain is misleading as it points at zlib as the culprit or something related to the compressed stream.

CensoredUsername commented 6 months ago

Adding an extra check around there cannot hurt as well then. I'll make some changes to also catch that.

CensoredUsername commented 5 months ago

This should all be fixed now.