MagicStack / MagicStack/asyncpg

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

Abierto Apto para principiantes
#1,328 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Lenguaje dominante
Python
Estrellas
8.1k
Forks
468
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

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

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Empieza en asyncpg/protocol/record/recordobj.c, en record_get(), y luego inspecciona tests/test_record.py::test_record_get. Compila la extensión con el comando documentado de setup.py y reproduce las llamadas con cero y tres argumentos usando el internal record helper. Se considera terminado cuando ambas llamadas no válidas generan TypeError sin que el proceso se bloquee y las pruebas de regresión pasan.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
c, postgresql, python
Área
backend, databases
Tipo de issue
Error
Dificultad
2/5
Tiempo estimado
1-3 horas
Estado de actividad
Tranquilo
Claridad
Bien especificado
Aptitud para principiantes
88/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.