Pwndbg: Bad unsigned casting

[GrosQuildu]

Description

pwndbg.memory.u returns signed integers (with minus - sign).

Steps to reproduce

#include <stdio.h>
#include <stdint.h>

int main(int argc, char const *argv[])
{
    uint64_t x = 0xb60ad86e8fb52ea8;
    printf("%p\n", &x);
    getc(stdin);
    return 0;
}

clang bad_u.c -g -o bad_u
gdb ./bad_u

pwndbg> x/xg 0x7fffffffab18
0x7fffffffab18: 0xb60ad86e8fb52ea8
pwndbg> python-interactive 
>>> pwndbg.memory.u(0x7fffffffab18)
-5329209239670542680

Idk why it doesn't break the pwndbg visibly. Found it running vis_heap_chunks on arbitrary addresses (the minus were printed in few places).

My setup

GNU gdb (Ubuntu 8.1-0ubuntu3.2) 8.1.0.20180409-git
python: 3.6.9 (default, Nov  7 2019, 10:44:02)
pwndbg: dev branch

[disconnect3d]

EDIT: To see what is the issue, scroll to https://github.com/pwndbg/pwndbg/issues/726#issuecomment-600760345

I can reproduce this

pwndbg> x/xg 0x7fffffffab18
0x7fffffffab18: 0xb60ad86e8fb52ea8
pwndbg> pi pwndbg.typeinfo.uint64.name
'unsigned long long'
pwndbg> pi pwndbg.typeinfo.uint64.pointer().name
pwndbg> pi gdb.Value(0x7fffffffab18).cast(pwndbg.typeinfo.uint64.pointer())
<gdb.Value object at 0x7fb472434a70>
pwndbg> pi int(gdb.Value(0x7fffffffab18).cast(pwndbg.typeinfo.uint64.pointer()))
140737488333592
pwndbg> pi hex(140737488333592)
'0x7fffffffab18'
pwndbg> pi int(gdb.Value(0x7fffffffab18).cast(pwndbg.typeinfo.uint64.pointer()).dereference())
-5329209239670542680
pwndbg> pi gdb.Value(0x7fffffffab18).cast(pwndbg.typeinfo.uint64.pointer()).dereference().type.name
'unsigned long long'

I am not sure, but if I remember correctly we had some discussions about this behaviour...

[disconnect3d]

Here, pure/raw GDB equivalent:

pi int(gdb.Value(0x7fffffffab18).cast(gdb.lookup_type('unsigned long long').pointer()).dereference())

[disconnect3d]

And I can confirm this being a Pwndbg bug, not a GDB behavior:

(gdb) x/xg 0x7fffffffe190
0x7fffffffe190: 0xb60ad86e8fb52ea8

(gdb) pi int(gdb.Value(0x7fffffffe190).cast(gdb.lookup_type('unsigned long long').pointer()).dereference())
13117534834039008936

[disconnect3d]

Our inthook fials again :(

pwndbg> pi pwndbg.inthook.xint.__new__(pwndbg.inthook.xint, 13117534834039008936)
xint.__new__ <class 'pwndbg.inthook.xint'> 13117534834039008936 () {}
13117534834039008936

pwndbg> pi pwndbg.inthook.xint.__new__(pwndbg.inthook.xint, gdb.Value(13117534834039008936))
xint.__new__ <class 'pwndbg.inthook.xint'> 13117534834039008936 () {}
-5329209239670542680

EDIT: Well actually... I am not sure if that is the case, but maaaybe.

EDIT2: Yes.

pwndbg> pi int(gdb.Value(0x7fffffffab18).cast(gdb.lookup_type('unsigned long long').pointer()).dereference())
xint.__new__ <class 'pwndbg.inthook.xint'> 13117534834039008936 (type: <class 'gdb.Value'>) () {}
-5329209239670542680

[disconnect3d]

One idea: drop support for Python 2 completely and remove the inthook. Removing the inthook source code makes this case work _as expected_, heh.

@zachriggle thoughts about dropping Py2 support finally? GEF did that few months ago :P.

[disconnect3d]

Just so that all readers of this issue are on the same page, the problematic code is here:

class xint(with_metaclass(IsAnInt, builtins.int)):
    def __new__(cls, value, *a, **kw):
        if isinstance(value, gdb.Value):
            if pwndbg.typeinfo.is_pointer(value):
                value = value.cast(pwndbg.typeinfo.size_t)
            else:
                value = value.cast(pwndbg.typeinfo.ssize_t)

        # (...) - code handling other types, removed as not needed to describe the issue

        return _int(_int(value, *a, **kw))

This is our inthook implementation: we do it to support both PY2 and PY3 in GDB. Why? Because int(gdb.Value(...)) on GDB with PY2 didn't work.

So we hooked int globally via globals()['int'] = xint and globals()['long'] = xint (in py2) and handled "proper conversions to int" by ourselves.

This code has been changed through time, to e.g. add support for Python 3 enum classes or to fix some bugs that were found in the meantime.

The bug can be fixed by changing this line:

-                 value = value.cast(pwndbg.typeinfo.ssize_t)
+                return _int.__new__(cls, value, *a, **kw)

However, I have no idea if it doesn't break anything else...

Given that, and the fact we are in 2020, maybe it is time to drop Py2 support and go with Py3 only.

[zachriggle]

As long as we do one last release for Python2 support, I'm down for dropping it in the next release

[disconnect3d]

Should be fixed by #747, we can proceed with a last release of Py2 and delete this file soon.