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

Gaurd against pop from empty block stack #415

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dennis-doyensec
Copy link

@dennis-doyensec dennis-doyensec commented Dec 11, 2023

I got a segfault when a block stack pop, when the block stack was already empty. This pull prevents emptying the block stack unless you are on the last instruction of the file. Or this is what I intended at least, I am new to this code base and pyc files in general.

This will prevent segfault seen found in issue #387, the provided pyc file there will decompile all the way with this pull. However, it can't be recompiled correctly unless it's a illegitimate pyc file. So I don't know if you would consider this a "fix". I guess it just changes the problem to something new.

I had a different crash that this pull prevents. It is a legitimate pyc file but a different python version.

@greenozon
Copy link
Contributor

I guess one need to understand the root reason...
such kind of fix is a blurring real issue under the hood..
IMHO

@dennis-doyensec
Copy link
Author

Sorry for late response.

In most cases I would agree with you 100%. I don't like half-fixes that hide
bugs. In this case, I think it depends on what you want this tool to do.

If you want pycdc to decompile legitimately created pyc files, stick with what
you have. Although, I would rather get an exit with a good warning, then a
segfault.

If you want this tool to also be useful for malware analysis, I would make a
change. Probably a better change then what I proposed. This bug can be used to
make a pyc file that can't be decompiled but can be loaded.

The following pyc will run fine despite being invalid.

$ python3 -c 'import fun'      ## loads fun.pyc without any problems
Never seen

$ pycdc fun.pyc                ## pycdc with this patch (see warning)
# Source Generated with Decompyle++
# File: fun.pyc (Python 3.11)

Warning: Refusing to pop last block when there is more code to parse pos: 18 OP: 8c
if 1 == 4:
    pass
None()
print('Never seen')

$ pycdc fun.pyc                ## latest pycdc (2da061fc985) crashes, no useful output
# Source Generated with Decompyle++
# File: fun.pyc (Python 3.11)

[1]    13838 segmentation fault  pycdc fun.pyc

$ pycdas fun.pyc              ## assembly shows how the trick worked (invalid dead code)
fun.pyc (Python 3.11)
[Code]
    File Name: fun.py
    Object Name: <module>
    Qualified Name: <module>
    Arg Count: 0
    Pos Only Arg Count: 0
    KW Only Arg Count: 0
    Stack Size: 3
    Flags: 0x00000000
    [Names]
        'print'
    [Locals+Names]
    [Constants]
        1
        4
        'Never seen'
        None
    [Disassembly]
        0       RESUME                          0
        2       LOAD_CONST                      0: 1
        4       LOAD_CONST                      1: 4
        6       COMPARE_OP                      2 (==)
        12      POP_JUMP_FORWARD_IF_FALSE       10 (to 34)
        14      JUMP_FORWARD                    0 (to 16)
        16      JUMP_FORWARD                    0 (to 18)
        18      PRECALL                         0
        22      CALL                            0
        32      POP_TOP
        34      PUSH_NULL
        36      LOAD_NAME                       0: print
        38      LOAD_CONST                      2: 'Never seen'
        40      PRECALL                         1
        44      CALL                            1
        54      POP_TOP
        56      LOAD_CONST                      3: None
        58      RETURN_VALUE

@greenozon
Copy link
Contributor

@dennis-doyensec
what was the original python code for the test case above?

at least two consecutive jumps looks very suspicious, isn't it?

        14      JUMP_FORWARD                    0 (to 16)
        16      JUMP_FORWARD                    0 (to 18)

I tried this

if 1 == 4:
    pass
print('Never seen')
```
no issues with both pycdas as well as pycdc

@dennis-doyensec
Copy link
Author

Sorry it's been a bit. I don't have that file anymore.

Those jumps are where I made a binary patch. There should be a PUSH there. Without the push, the stack was empty when a pop occurred. Python's interpreter was fine because it never took that 1 == 4 branch.

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.

3 participants