Gallopsled / pwntools

CTF framework and exploit development library
http://pwntools.com
Other
11.69k stars 1.67k forks source link

flat() spins the cpu and never returns #2423

Open MarcoPellero opened 1 week ago

MarcoPellero commented 1 week ago

Running flat({1: 0}, word_size=8) spins the cpu and never returns. In general, running flat({X: Y}, word_size=Z) with X >= 2^(Z-8) (for example flat({256: 0}, word_size=16) hangs.

I've tracked the issue to the _fit() function in pwnlib.util.packing.py. It calculates a large_key=2**(context.word_size-8) (context.word_size is set to our parameter), iterates the dictionary given to flat(), and when given a numeric key it compares it to large_key and calls an internal function fill() if the dict key is higher or equal to large_key.

This fill() function keeps generating filler bytes (using the filler generator given to flat()) until it encounters the packed version of our numeric key. So, when using {1: 0} with word_size=8, it will keep generating bytes until it generates b'\x01, and when using {256: 0} with word_size=16, it will keep going until b'\x00\x01. With the default filler (de_bruijn()) this never happens, since these keys aren't in its default alphabet. it also spins with filler=b'\x00, or with any filler that doesn't generate that packed key.

I can't understand the logic behind using this fill() function. These generated bytes are also thrown away, the only reason to genreate them are to find the offset of the key in the final payload according to the comments, I think, but the final offset should be equal to the original key itself. By changing:

def _fit(pieces, preprocessor, packer, filler, stacklevel=1):
    ...
    for k, v in pieces.items():
        if isinstance(k, six.integer_types):
            if k >= large_key:
                k = fill(pack(k))
    ...

To:

def _fit(pieces, preprocessor, packer, filler, stacklevel=1):
    ...
    for k, v in pieces.items():
        if isinstance(k, six.integer_types):
            pass
    ...

The function stops spinning on these bad inputs, but this isn't the only case that fill() is called, there's another one, and since I don't understand the reason why this code was used in the first place I'm not confident that it's an actual fix.

I'm running pwntools 4.12.0, but when looking at the commit history I didn't see recent commits that affected this function. I haven't tested it with older versions.