MagicStack / MagicStack/asyncpg

`Record.get()` with invalid positional argument count segfaults

Offen Anfängerfreundlich
#1,328 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

Vorherrschende Sprache
Python
Sterne
8.1k
Forks
468
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

Calling `asyncpg.Record.get()` with an invalid number of positional arguments can crash the Python process instead of raising `TypeError`.

Confirmed crashing calls:

- `record.get()`
- `record.get("a", 2, 3)`

Valid and separately handled cases behave as expected:

- `record.get("a")` returns the value.
- `record.get("a", default=2)` raises `TypeError: Record.get() takes no keyword arguments`.

## Affected Component

- File: `asyncpg/protocol/record/recordobj.c`
- Function: `record_get()`
- Method exposed as: `asyncpg.Record.get`
- Observed commit: `db8ecc2a38e16fb0c090aef6f5506547c2831c24`

## Impact

This is a native crash / process-level denial of service in the CPython extension. It is not a PostgreSQL wire-level remote issue by itself; it requires same-process Python code to call `Record.get()` with an invalid positional argument count. This can still matter for applications that expose generic object dispatch, plugins, scripting hooks, template helpers, or RPC-style method invocation over returned records.

## Root Cause

In `record_get()`, the invalid positional argument-count branch sets a Python exception but continues execution:

```c
if (nargs == 2) {
key = args[0];
defval = args[1];
} else if (nargs == 1) {
key = args[0];
} else {
PyErr_Format(PyExc_TypeError,
"Record.get() expected 1 or 2 arguments, got %zd",
nargs);
}
```

`key` is not initialized in that branch. The function then reaches:

```c
res = record_item_by_name((ApgRecordObject *)self, key, &val);
```

As a result, an uninitialized `PyObject *key` is passed to `record_item_by_name()`, causing a native crash.

The release build also emits:

```text
asyncpg/protocol/record/recordobj.c:702:11: warning: 'key' may be used uninitialized [-Wmaybe-uninitialized]
```

## Steps to Reproduce

Build asyncpg from source:

```bash
git submodule update --init --recursive
python setup.py build_ext --inplace
```

Minimal repro without requiring a PostgreSQL server, using the same internal record helper used by `tests/test_record.py`:

```bash
PYTHONPATH=. python -u - <<'PY'
from asyncpg.protocol.protocol import _create_record as Record

r = Record({"a": 0}, (1,))
print("before")
r.get()
print("after")
PY
```

A three-positional-argument variant also crashes:

```bash
PYTHONPATH=. python -u - <<'PY'
from asyncpg.protocol.protocol import _create_record as Record

r = Record({"a": 0}, (1,))
print("before")
r.get("a", 2, 3)
print("after")
PY
```

A public API variant can be reproduced by fetching any row and then calling the invalid method form:

```python
import asyncio
import asyncpg

async def main():
conn = await asyncpg.connect()
try:
row = await conn.fetchrow("select 1 as a")
row.get()
finally:
await conn.close()

asyncio.run(main())
```

## Expected Result

Invalid positional argument counts should raise a Python exception, for example:

```text
TypeError: Record.get() expected 1 or 2 arguments, got 0
```

and:

```text
TypeError: Record.get() expected 1 or 2 arguments, got 3
```

## Actual Result

On a release build, both invalid calls segfault:

```text
before no args
Segmentation fault (core dumped)
```

```text
before three args
Segmentation fault (core dumped)
```

Local verification exited with code `139` for both `r.get()` and `r.get("a", 2, 3)`.

With ASAN, the invalid argument-count path produced:

```text
AddressSanitizer:DEADLYSIGNAL
ERROR: AddressSanitizer: SEGV on unknown address
```

## Suggested Fix

Return immediately after setting the argument-count error:

```c
} else {
PyErr_Format(PyExc_TypeError,
"Record.get() expected 1 or 2 arguments, got %zd",
nargs);
return NULL;
}
```

It would also be useful to add regression coverage to `tests/test_record.py::test_record_get`:

```python
with self.assertRaises(TypeError):
r.get()

with self.assertRaises(TypeError):
r.get("a", 2, 3)
```

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Beginne in asyncpg/protocol/record/recordobj.c bei record_get() und untersuche anschließend tests/test_record.py::test_record_get. Baue die Extension mit dem dokumentierten setup.py-Befehl und reproduziere die Aufrufe mit null und drei Argumenten mithilfe des internen record helper. Erledigt ist die Aufgabe, wenn beide ungültigen Aufrufe TypeError auslösen, ohne dass der Prozess abstürzt, und die Regressionstests erfolgreich durchlaufen.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
c, postgresql, python
Bereich
backend, databases
Issue-Typ
Bug
Schwierigkeit
2/5
Geschätzter Aufwand
1-3 Stunden
Aktivitätsstatus
Ruhig
Klarheit
Klar beschrieben
Anfängerfreundlichkeit
88/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.