MagicStack / MagicStack/asyncpg

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

Open Beginner friendly
#1,328 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
8.1k
Forks
468
PR merge metrics
No merged PRs in 30d

Description

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:

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:

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:

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

Steps to Reproduce

Build asyncpg from source:

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:

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:

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:

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:

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

and:

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

Actual Result

On a release build, both invalid calls segfault:

before no args
Segmentation fault (core dumped)
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:

AddressSanitizer:DEADLYSIGNAL
ERROR: AddressSanitizer: SEGV on unknown address

Suggested Fix

Return immediately after setting the argument-count error:

} 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:

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

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

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in asyncpg/protocol/record/recordobj.c at record_get(), then inspect tests/test_record.py::test_record_get. Build the extension with the documented setup.py command and reproduce the zero- and three-argument calls using the internal record helper. Done means both invalid calls raise TypeError without a process crash, with regression coverage passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, postgresql, python
Domain
backend, databases
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.