MagicStack / MagicStack/asyncpg

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

Aperta Adatta ai principianti
#1,328 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Lingua principale
Python
Stelle
8.1k
Fork
468
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

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)
```

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Direzione di ricerca

Inizia in asyncpg/protocol/record/recordobj.c, in record_get(), quindi esamina tests/test_record.py::test_record_get. Compila l’estensione con il comando setup.py documentato e riproduci le chiamate con zero e tre argomenti usando l’internal record helper. Il lavoro è completato quando entrambe le chiamate non valide sollevano TypeError senza causare un arresto anomalo del processo e i test di regressione passano.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
c, postgresql, python
Ambito
backend, databases
Tipo di issue
Bug
Difficoltà
2/5
Tempo stimato
1-3 ore
Stato di attività
Tranquilla
Chiarezza
Specificata chiaramente
Idoneità per principianti
88/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.