Make cyclic_find work with large int values

[justinsteven]

This resolves #1965 by making cyclic_find() work with integers larger than what could fit in n bytes.

It does so by truncating the integer value, similarly to how cyclic_find() has been truncating long bytes-type values. It honours context.endianness in its truncation.

I also tidied up the warning in the case of overly long bytes-type values. It was printing 4 instead of pulling from n.

Demo

Old behavior:

>>> import pwn

>>> pwn.cyclic_find(b"baaa")
4

>>> pwn.cyclic_find(b"baaacaaa")
[!] cyclic_find() expects 4-byte subsequences by default, you gave b'baaacaaa'
    Unless you specified cyclic(..., n=8), you probably just want the first 4 bytes.
    Truncating the data at 4 bytes.  Specify cyclic_find(..., n=8) to override this.
4

>>> pwn.cyclic_find(pwn.u32(b"baaa"))
4

>>> pwn.cyclic_find(pwn.u64(b"baaacaaa"))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/pwntools/pwnlib/context/__init__.py", line 1547, in setter
    return function(*a, **kw)
  File "/opt/pwntools/pwnlib/util/cyclic.py", line 211, in cyclic_find
    subseq = packing.pack(subseq, bytes=n)
  File "/opt/pwntools/pwnlib/util/packing.py", line 149, in pack
    raise ValueError("pack(): number does not fit within word_size [%i, %r, %r]" % (0, number, limit))
ValueError: pack(): number does not fit within word_size [0, 7016996773883371874, 4294967296]

Even setting endian.context doesn't resolve it:

>>> with pwn.context.local(arch="amd64"):
...   pwn.cyclic_find(pwn.u64(b"baaacaaa"))
...
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "/opt/pwntools/pwnlib/context/__init__.py", line 1547, in setter
    return function(*a, **kw)
  File "/opt/pwntools/pwnlib/util/cyclic.py", line 211, in cyclic_find
    subseq = packing.pack(subseq, bytes=n)
  File "/opt/pwntools/pwnlib/util/packing.py", line 149, in pack
    raise ValueError("pack(): number does not fit within word_size [%i, %r, %r]" % (0, number, limit))
ValueError: pack(): number does not fit within word_size [0, 7016996773883371874, 4294967296]

New behavior:

>>> import pwn

>>> pwn.cyclic_find(b"baaa")
4

>>> pwn.cyclic_find(b"baaacaaa")
[!] cyclic_find() expected a 4-byte subsequence, you gave b'baaacaaa'
    Unless you specified cyclic(..., n=8), you probably just want the first 4 bytes.
    Truncating the data at 4 bytes.  Specify cyclic_find(..., n=8) to override this.
4

>>> pwn.cyclic_find(pwn.u32(b"baaa"))
4

>>> pwn.cyclic_find(pwn.u64(b"baaacaaa"))
[!] cyclic_find() expected an integer argument < 0x100000000, you gave 0x6161616361616162
    Unless you specified cyclic(..., n=8), you probably just want the first 4 bytes.
    Truncating the data at 4 bytes.  Specify cyclic_find(..., n=8) to override this.
4

>>> pwn.cyclic_find(pwn.u64(b"baaacaaa", endian="big"), endian="big")
[!] cyclic_find() expected an integer argument < 0x100000000, you gave 0x6261616163616161
    Unless you specified cyclic(..., n=8), you probably just want the first 4 bytes.
    Truncating the data at 4 bytes.  Specify cyclic_find(..., n=8) to override this.
4

While I filed #1965 as a bug, I'm not sure this warrants backporting to stable. It is behaviour that has been completely blowing up on users so far, so they'd be used to routing around it.

[justinsteven]

The CI failure doesn't look related to this PR

[zachriggle]

Thanks for the PR, but this situation is already handled and the code is behaving correctly.

Set context.cyclic_size or manually specify the size when calling cyclic_find.

>>> cyclic_find(u64(b'baaacaaa'), n=8)
3515208

>>> context.bytes = 8
>>> context.cyclic_size = 8
>>> cyclic_find(u64(b'baaacaaa'))
3515208

I do agree we need a better error message, but the behavior is correct, and the default cyclic size is ALWAYS four bytes. It looks in your case, you do indeed have a 4-byte cyclic pattern, but are decoding it incorrectly.

To save confusion in the future, just pass the bytes directly to cyclic_find and it will do the right thing.

>>> from pwn import *
>>> cyclic_find(b'baaacaaa')
[!] cyclic_find() expects 4-byte subsequences by default, you gave b'baaacaaa'
    Unless you specified cyclic(..., n=8), you probably just want the first 4 bytes.
    Truncating the data at 4 bytes.  Specify cyclic_find(..., n=8) to override this.
4

[justinsteven]

Thanks @zachriggle

While it might be a minor issue, the current working-as-intended behaviour can be a nuisance in some cases. #1965 shows some of these, but perhaps they're not presented as enough of a real-world problem.

It looks in your case, you do indeed have a 4-byte cyclic pattern, but are decoding it incorrectly.

What I'm speaking of is the case where I've used, say, pwn.cyclic() or pwn.flat(). Neither of these respect context.arch or context.bytes to set the n value - and for pwn.flat() you can't set the n value unless you do pwn.flat({32: 0x41414141}, filler=pwn.de_bruijn(n=8)) which isn't easy to remember to do.

I've then got a crash with a register being, say, 0x6b6161666b616165. I'd like to pass this to cyclic_find().

It currently doesn't work:

>>> with pwn.context.local(arch="amd64"):
...   pwn.cyclic_find(0x6b6161666b616165)
...
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "/opt/pwntools/pwnlib/context/__init__.py", line 1547, in setter
    return function(*a, **kw)
  File "/opt/pwntools/pwnlib/util/cyclic.py", line 211, in cyclic_find
    subseq = packing.pack(subseq, bytes=n)
  File "/opt/pwntools/pwnlib/util/packing.py", line 149, in pack
    raise ValueError("pack(): number does not fit within word_size [%i, %r, %r]" % (0, number, limit))
ValueError: pack(): number does not fit within word_size [0, 7737572727315325285, 4294967296]

Curiously we do have support for overly long bytes subsequences:

>>> with pwn.context.local(arch="amd64"):
...   pwn.cyclic_find(pwn.p64(0x6b6161666b616165))
...
[!] cyclic_find() expects 4-byte subsequences by default, you gave b'eaakfaak'
    Unless you specified cyclic(..., n=8), you probably just want the first 4 bytes.
    Truncating the data at 4 bytes.  Specify cyclic_find(..., n=8) to override this.
1016

To get around this API inconsistency and to successfully use pwn.cyclic_find() I'd currently have to:

• Use an unnecessary pwn.p64() to turn it into bytes to take advantage of the fact that cyclic_find() does work with 8 byte bytes subsequences (see above) - pwn.cyclic_find(pwn.p64(0x6b6161666b616165))
• Manually trim the int value so that it's a 32-bit value (error prone) - pwn.cyclic_find(0x666b616165) - oops, I messed it up 🙃
• Mask it with 0xffffffff so that it's a 32-bit value (not intuitive code) - pwn.cyclic_find(0x6b6161666b616165 & 0xffffffff)

None of which are terribly clean. I'd rather pwntools trim the overly large int for me, and warn loudly when it does so, just as it's currently doing for overly long bytes subsequences.

To make things simpler, this PR allows users to do:

>>> pwn.cyclic_find(0x6b6161666b616165)
[!] cyclic_find() expected an integer argument < 0x100000000, you gave 0x6b6161666b616165
    Unless you specified cyclic(..., n=8), you probably just want the first 4 bytes.
    Truncating the data at 4 bytes.  Specify cyclic_find(..., n=8) to override this.
1016

Sure it gives an ugly warning, and perhaps it's something that people "shouldn't" be doing, but it does bring the handling of large ints into alignment with how we're handling overly long bytes.

If catering to this use case isn't something that pwntools is interested in doing, then that's ok. It sure would shave off a rough edge for me though.

But on the other hand, does this PR break something I'm not aware of?

[zachriggle]

To make things simpler, this PR allows users to do:

pwn.cyclic_find(0x6b6161666b616165)
[!] cyclic_find() expected an integer argument < 0x100000000, you gave 0x6b6161666b616165
Unless you specified cyclic(..., n=8), you probably just want the first 4 bytes.
Truncating the data at 4 bytes. Specify cyclic_find(..., n=8) to override this.
1016

Then I completely misunderstood the intent of this PR, and my diatribe can be safely ignored.

Everything sounds good to me, though I'd prefer <= 0xffffffff vs < 0x100000000 but that's just stylistic.

[justinsteven]

I'd prefer <= 0xffffffff vs < 0x100000000 but that's just stylistic

Ah yep, totally agree. Updated.

>>> pwn.cyclic_find(0x6b6161666b616165)
[!] cyclic_find() expected an integer argument <= 0xffffffff, you gave 0x6b6161666b616165
    Unless you specified cyclic(..., n=8), you probably just want the first 4 bytes.
    Truncating the data at 4 bytes.  Specify cyclic_find(..., n=8) to override this.
1016