gdb.corefile: Run gdb directly to prevent detach race (#2787)

* gdb.corefile: Run gdb directly to prevent detach race

`attach()` waits for the debugger to attach by polling the `TracerPid` regularly. `corefile()` runs `detach` in gdb after dumping the core, so the debugger might attach and detach between two `TracerPid` polls and `wait_for_debugger()` thinks gdb exited before attaching and returns `None` as gdb's pid is no longer valid.

Later `corefile()` ran `psutil.Process(pid).wait()` with `pid = None` which effectively waited on the Python process itself to exit.
This caused a hang in the doctest CI sometimes, which was killed by the watchdog timer eventually.

Run `gdb` directly and just observe the exit code instead of relying on the more advanced `attach` logic for longer running debug sessions.

* Update CHANGELOG

.github/workflows/ci.yml

@@ -150,6 +150,8 @@ jobs:
         pip install --upgrade wheel build
         pip install --upgrade flake8 appdirs
         pip install --upgrade --editable .
+        # Pin to known working version until 2.1.5 is out. https://github.com/unicorn-engine/unicorn/issues/2134
+        pip install 'unicorn==2.1.2'
 
     - name: Sanity checks
       run:  PWNLIB_NOTERM=1 python -bb -c 'from pwn import *; print(pwnlib.term.term_mode)'
@@ -285,7 +287,7 @@ jobs:
       run: |
         pip install --upgrade pip
         pip install --upgrade --editable .
-    
+
     - name: Sanity checks
       run: |
         python -bb -c 'from pwn import *'

.github/workflows/pylint.yml

@@ -24,7 +24,7 @@ jobs:
     - name: PyLint
       run: |
         set -x
-        pip install pylint
+        pip install 'pylint<4'
         pip install --upgrade -e .
         run_pylint() { pylint --exit-zero --errors-only pwnlib -f parseable | cut -d ' ' -f2- | sed 's/line [0-9]\+/line XXXX/g'; }
         run_pylint > current.txt

CHANGELOG.md

@@ -129,6 +129,22 @@ The table below shows which release corresponds to each branch, and what date th
 [2610]: https://github.com/Gallopsled/pwntools/pull/2610
 [2615]: https://github.com/Gallopsled/pwntools/pull/2615
 
+## 4.15.1
+
+- [#2694][2694] fix: pad bytes fields to correct field size in FileStructure
+- [#2701][2701] Fix `adb._build_date()` crash on devices with non-standard locale date strings
+- [#2721][2721] Allow running with Unicorn 2.1.[34] but throw when emulating MIPS
+- [#2773][2773] Disable debuginfod doctests in libcdb.unstrip_libc
+- [#2772][2772] fix(packing): `dd()` no longer crashes on a `bytearray` destination
+- [#2787][2787] gdb.corefile: Run gdb directly to prevent detach race
+
+[2694]: https://github.com/Gallopsled/pwntools/pull/2694
+[2701]: https://github.com/Gallopsled/pwntools/pull/2701
+[2721]: https://github.com/Gallopsled/pwntools/pull/2721
+[2773]: https://github.com/Gallopsled/pwntools/pull/2773
+[2772]: https://github.com/Gallopsled/pwntools/pull/2772
+[2787]: https://github.com/Gallopsled/pwntools/pull/2787
+
 ## 4.15.0 (`stable`)
 
 - [#2508][2508] Ignore a warning when compiling with asm on nix
@@ -193,7 +209,7 @@ The table below shows which release corresponds to each branch, and what date th
 [2612]: https://github.com/Gallopsled/pwntools/pull/2612
 [2624]: https://github.com/Gallopsled/pwntools/pull/2624
 
-## 4.14.1 (`stable`)
+## 4.14.1
 
 - [#2451][2451] Show symbols defined to value 0 (start of file)
 - [#2533][2533] Fix installation on Python 3.5 and lower
@@ -644,7 +660,7 @@ In memoriam — [Zach Riggle][zach] — long time contributor and maintainer of
 [fecf9f]: http://github.com/Gallopsled/pwntools/commit/fecf9f
 [1454]: https://github.com/Gallopsled/pwntools/pull/1454
 
-## 4.1.7 (`stable`)
+## 4.1.7
 
 - [#1615][1615] Fix aarch64 pushstr and pushstr_array
 

docs/requirements.txt

@@ -19,6 +19,6 @@ psutil
 requests>=2.5.1
 ropgadget>=5.3
 sphinx==1.8.6; python_version<'3'
-sphinx>=7.0.0; python_version>='3'
+sphinx>=8.1.3, <9; python_version>='3'
 sphinx_rtd_theme
 sphinxcontrib-autoprogram<=0.1.5

docs/source/intro.rst

@@ -34,6 +34,8 @@ This exposes a standard interface to talk to processes, sockets, serial ports,
 and all manner of things, along with some nifty helpers for common tasks.
 For example, remote connections via :mod:`pwnlib.tubes.remote`.
 
+::
+
     >>> conn = remote('ftp.ubuntu.com',21)
     >>> conn.recvline() # doctest: +ELLIPSIS
     b'220 ...'

pwnlib/adb/adb.py

@@ -1275,8 +1275,28 @@ properties = Property()
 
 def _build_date():
     """Returns the build date in the form YYYY-MM-DD as a string"""
+    import datetime
+
+    # Use ro.build.date.utc (integer epoch seconds) which is set by the
+    # AOSP build system and available on all standard Android devices.
+    # This avoids ro.build.date which is locale-dependent and can contain
+    # non-ASCII characters that dateutil cannot parse.  See #2513.
+    utc = getprop('ro.build.date.utc')
+    if utc and utc.strip().isdigit():
+        try:
+            as_datetime = datetime.datetime.fromtimestamp(int(utc.strip()), dateutil.tz.UTC)
+            return as_datetime.strftime('%Y-%b-%d')
+        except (OSError, OverflowError, ValueError):
+            pass
+
+    # Fallback for non-standard builds missing ro.build.date.utc.
     as_string = getprop('ro.build.date')
-    as_datetime =  dateutil.parser.parse(as_string)
+    if not as_string:
+        return ''
+    try:
+        as_datetime = dateutil.parser.parse(as_string)
+    except (ValueError, OverflowError):
+        return as_string
     return as_datetime.strftime('%Y-%b-%d')
 
 def find_ndk_project_root(source):

pwnlib/elf/elf.py

@@ -764,7 +764,7 @@ class ELF(ELFFile):
             (see :attr:`.ELF.libs`).
         """
         for lib in self.libs:
-            if '/libc.' in lib or '/libc-' in lib:
+            if ('/libc.' in lib or '/libc-' in lib) and lib != self.path:
                 return ELF(lib, self._print_checksec)
 
     def _populate_libraries(self):

pwnlib/elf/plt.py

@@ -78,6 +78,13 @@ def __ensure_memory_to_run_unicorn():
 def prepare_unicorn_and_context(elf, got, address, data):
     import unicorn as U
 
+    if elf.arch in ('mips', 'mips64'):
+        # Unicorn 2.1.3 and 2.1.4 have a crashing bug when trying to emulate MIPS.
+        # https://github.com/unicorn-engine/unicorn/issues/2134
+        if U.UC_VERSION_MAJOR == 2 and U.UC_VERSION_MINOR == 1 and U.UC_VERSION_PATCH in (3, 4):
+            raise OSError("Unicorn Engine 2.1.3 and 2.1.4 have a crashing bug when emulating MIPS, please install an earlier or later version: https://github.com/unicorn-engine/unicorn/issues/2134")
+
+
     __ensure_memory_to_run_unicorn()
 
     # Instantiate the emulator with the correct arguments for the current

pwnlib/filepointer.py

@@ -156,6 +156,18 @@ class FileStructure(object):
          _wide_data: 0xdeadbeef
          unknown2: 0x0
          vtable: 0x0}
+
+        Bytes fields are padded to the correct field size.
+        For example, unknown2 is 56 bytes on i386, so a short value gets
+        zero-padded to 56, not to context.bytes (4):
+
+        >>> context.clear(arch='i386')
+        >>> fileStr2 = FileStructure(null=0)
+        >>> fileStr2.vtable = 0x561859f0
+        >>> old_len = len(bytes(fileStr2))
+        >>> fileStr2.unknown2 = b'AB'
+        >>> len(bytes(fileStr2)) == old_len
+        True
     """
 
     vars_=[]
@@ -186,7 +198,7 @@ class FileStructure(object):
         structure = b''
         for val in self.vars_:
             if isinstance(getattr(self, val), bytes):
-                structure += getattr(self, val).ljust(context.bytes, b'\x00')
+                structure += getattr(self, val).ljust(self.length[val], b'\x00')
             else:
                 if self.length[val] > 0:
                     structure += pack(getattr(self, val), self.length[val]*8)
@@ -215,7 +227,7 @@ class FileStructure(object):
         structure = b''
         for val in self.vars_:
             if isinstance(getattr(self, val), bytes):
-                structure += getattr(self, val).ljust(context.bytes, b'\x00')
+                structure += getattr(self, val).ljust(self.length[val], b'\x00')
             else:
                 structure += pack(getattr(self, val), self.length[val]*8)
             if val == v:

pwnlib/gdb.py

@@ -143,7 +143,6 @@ from __future__ import division
 
 import os
 import platform
-import psutil
 import random
 import re
 import six
@@ -1522,28 +1521,33 @@ def corefile(process):
                       'contain all of the data in the process.\n'
                       'Upgrade to GDB >= 7.11 for better core-dumps.' % binary())
 
-    # This is effectively the same as what the 'gcore' binary does
-    gdb_args = ['-batch',
-                '-q',
-                '-nx',
-                '-ex', 'set pagination off',
-                '-ex', 'set height 0',
-                '-ex', 'set width 0',
-                '-ex', 'set use-coredump-filter on',
-                '-ex', 'generate-core-file %s' % corefile_path,
-                '-ex', 'detach']
+    # This is effectively the same as what the 'gcore' binary does.
+    # Run GDB directly instead of going through attach(), which has to
+    # race GDB to observe the tracer before it already detached again.
+    cmd = [binary(),
+           '-batch',
+           '-q',
+           '-nx',
+           '-ex', 'set pagination off',
+           '-ex', 'set height 0',
+           '-ex', 'set width 0',
+           '-ex', 'set use-coredump-filter on',
+           '-ex', 'attach %d' % process.pid,
+           '-ex', 'generate-core-file %s' % corefile_path,
+           '-ex', 'detach']
 
-    with context.local(terminal = ['sh', '-c']):
-        with context.quiet:
-            pid = attach(process, gdb_args=gdb_args)
-            log.debug("Got GDB pid %d", pid)
-            try:
-                psutil.Process(pid).wait()
-            except psutil.Error:
-                pass
+    with context.quiet:
+        gdb = tubes.process.process(cmd, stdout=tubes.process.PIPE)
+        gdb.wait_for_close(timeout=60)
+        timed_out = gdb.poll() is None
+        output = gdb.recvallS(timeout=0)
+
+    if timed_out:
+        log.error("GDB did not finish dumping process %d within 60 seconds", process.pid)
 
     if not os.path.exists(corefile_path):
-        log.error("Could not generate a corefile for process %d", process.pid)
+        log.error("Could not generate a corefile for process %d (maybe check /proc/sys/kernel/yama/ptrace_scope)\n%s",
+                  process.pid, output)
 
     return elf.corefile.Core(corefile_path)
 

pwnlib/libcdb.py

@@ -354,15 +354,15 @@ def unstrip_libc(filename):
         >>> libc = ELF(filename)
         >>> 'main_arena' in libc.symbols
         False
-        >>> unstrip_libc(filename)
+        >>> unstrip_libc(filename) # doctest: +SKIP
         True
         >>> libc = ELF(filename)
-        >>> hex(libc.symbols.main_arena)
+        >>> hex(libc.symbols.main_arena) # doctest: +SKIP
         '0x219c80'
         >>> unstrip_libc(pwnlib.data.elf.get('test-x86'))
         False
         >>> filename = search_by_build_id('d1704d25fbbb72fa95d517b883131828c0883fe9', unstrip=True)
-        >>> 'main_arena' in ELF(filename).symbols
+        >>> 'main_arena' in ELF(filename).symbols # doctest: +SKIP
         True
     """
     if not which('eu-unstrip'):

pwnlib/term/readline.py

@@ -380,7 +380,7 @@ def readline(_size=-1, prompt='', float=True, priority=10):
     from pwnlib.term import term_mode
     if not term_mode:
         six.print_(prompt, end='', flush=True)
-        return getattr(sys.stdin, 'buffer', sys.stdin).readline(_size).rstrip(b'\n')
+        return force_to_bytes(getattr(sys.stdin, 'buffer', sys.stdin).readline(_size)).rstrip(b'\n')
     show_suggestions = False
     eof = False
     if prompt:

pwnlib/util/packing.py

@@ -988,6 +988,11 @@ def dd(dst, src, count = 0, skip = 0, seek = 0, truncate = False):
         ('H', 'e', 'l', 'l', 'o', b'?')
         >>> dd(list('Hello!'), (63,), skip = 5)
         ['H', 'e', 'l', 'l', 'o', b'?']
+        >>> buf = bytearray(b'Hello!')
+        >>> dd(buf, b'?', skip = 5)
+        bytearray(b'Hello?')
+        >>> buf
+        bytearray(b'Hello?')
         >>> _ = open('/tmp/foo', 'w').write('A' * 10)
         >>> dd(open('/tmp/foo'), open('/dev/zero'), skip = 3, count = 4).read()
         'AAA\\x00\\x00\\x00\\x00AAA'
@@ -1105,7 +1110,12 @@ def dd(dst, src, count = 0, skip = 0, seek = 0, truncate = False):
         dst = real_dst
 
     elif isinstance(dst, (list, bytearray)):
-        dst[skip : skip + len(src)] = list(map(p8, bytearray(src)))
+        # A bytearray slice takes a byte string, while a list slice takes the
+        # single-byte bytes objects that p8() returns.
+        if isinstance(dst, bytearray):
+            dst[skip : skip + len(src)] = src
+        else:
+            dst[skip : skip + len(src)] = list(map(p8, bytearray(src)))
         if truncate:
             while len(dst) > truncate:
                 dst.pop()

pyproject.toml

@@ -50,7 +50,7 @@ dependencies = [
     "psutil>=3.3.0",
     "intervaltree>=3.0",
     "sortedcontainers",
-    "unicorn>=2.0.1, !=2.1.3, !=2.1.4",  # see https://github.com/unicorn-engine/unicorn/issues/2134
+    "unicorn>=2.0.1",
     "six>=1.12.0",
     "rpyc",
     "colored_traceback<0.4; python_version < '3'",