Skip to content

Fix stack overflow when decoding deeply nested structures like arrays and maps #51

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Feb 17, 2025
1 change: 1 addition & 0 deletions data/semi_torture_nested_lists.dagcbor
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
�`������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������e$typerapp.bsky.feed.psot
File renamed without changes.
1 change: 1 addition & 0 deletions data/torture_nested_lists.dagcbor

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions data/torture_nested_maps.dagcbor

Large diffs are not rendered by default.

11 changes: 9 additions & 2 deletions pytests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,19 @@ def load_cbor_data_fixtures(dir_path: str) -> List[Tuple[str, Any]]:
return fixtures


def load_car_fixture(did: str, path: str) -> bytes:
def load_car_fixture(_: str, path: str) -> bytes:
if os.path.exists(path):
with open(path, 'rb') as f:
return f.read()

contents = urllib.request.urlopen(f'https://bsky.network/xrpc/com.atproto.sync.getRepo?did={did}').read()
url = 'https://github.com/MarshalX/python-libipld/releases/download/v1.0.0/test_huge_repo.car'

# Bsky team disabled the endpoint below.
# We could not rely on it anymore.
# Request forbidden by administrative rules (403 Forbidden).
# url = f'https://bsky.network/xrpc/com.atproto.sync.getRepo?did={did}'

contents = urllib.request.urlopen(url).read()
with open(path, 'wb') as f:
f.write(contents)

Expand Down
26 changes: 22 additions & 4 deletions pytests/test_dag_cbor.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@

from conftest import load_cbor_data_fixtures, load_json_data_fixtures

_ROUNDTRIP_DATA_DIR = os.path.join(os.path.dirname(__file__), '..', 'data', 'roundtrip')
_REAL_DATA_DIR = os.path.join(os.path.dirname(__file__), '..', 'data')
_FIXTURES_DATA_DIR = os.path.join(os.path.dirname(__file__), '..', 'data', 'fixtures')
_CIDS_DAG_CBOR_PATH = os.path.join(os.path.dirname(__file__), '..', 'data', 'torture_cids.dag-cbor')
_ROUNDTRIP_DATA_DIR = os.path.join(_REAL_DATA_DIR, 'roundtrip')
_FIXTURES_DATA_DIR = os.path.join(_REAL_DATA_DIR, 'fixtures')
_TORTURE_CIDS_DAG_CBOR_PATH = os.path.join(_REAL_DATA_DIR, 'torture_cids.dagcbor')
_TORTURE_NESTED_LISTS_DAG_CBOR_PATH = os.path.join(_REAL_DATA_DIR, 'torture_nested_lists.dagcbor')
_TORTURE_NESTED_MAPS_DAG_CBOR_PATH = os.path.join(_REAL_DATA_DIR, 'torture_nested_maps.dagcbor')


def _dag_cbor_encode(benchmark, data) -> None:
Expand Down Expand Up @@ -115,5 +117,21 @@ def test_dag_cbor_decode_fixtures(benchmark, data) -> None:


def test_dag_cbor_decode_torture_cids(benchmark) -> None:
dag_cbor = open(_CIDS_DAG_CBOR_PATH, 'rb').read()
dag_cbor = open(_TORTURE_CIDS_DAG_CBOR_PATH, 'rb').read()
benchmark(libipld.decode_dag_cbor, dag_cbor)


def test_recursion_limit_exceed_on_nested_lists() -> None:
dag_cbor = open(_TORTURE_NESTED_LISTS_DAG_CBOR_PATH, 'rb').read()
with pytest.raises(RecursionError) as exc_info:
libipld.decode_dag_cbor(dag_cbor)

assert 'in DAG-CBOR decoding' in str(exc_info.value)


def test_recursion_limit_exceed_on_nested_maps() -> None:
dag_cbor = open(_TORTURE_NESTED_MAPS_DAG_CBOR_PATH, 'rb').read()
with pytest.raises(RecursionError) as exc_info:
libipld.decode_dag_cbor(dag_cbor)

assert 'in DAG-CBOR decoding' in str(exc_info.value)
37 changes: 28 additions & 9 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,24 @@ fn string_new_bound<'py>(py: Python<'py>, s: &[u8]) -> Bound<'py, PyString> {
}
}


fn decode_dag_cbor_to_pyobject<R: Read + Seek>(
py: Python,
r: &mut R,
deep: usize,
depth: usize,
) -> Result<PyObject> {
unsafe {
if depth > ffi::Py_GetRecursionLimit() as usize {
PyErr::new::<pyo3::exceptions::PyRecursionError, _>(
"RecursionError: maximum recursion depth exceeded in DAG-CBOR decoding",
).restore(py);

return Err(anyhow!("Maximum recursion depth exceeded"));
}
}

let major = decode::read_major(r)?;
Ok(match major.kind() {
MajorKind::UnsignedInt => (decode::read_uint(r, major)?).to_object(py),
MajorKind::UnsignedInt => decode::read_uint(r, major)?.to_object(py),
MajorKind::NegativeInt => (-1 - decode::read_uint(r, major)? as i64).to_object(py),
MajorKind::ByteString => {
let len = decode::read_uint(r, major)?;
Expand All @@ -130,7 +139,7 @@ fn decode_dag_cbor_to_pyobject<R: Read + Seek>(
let ptr = ffi::PyList_New(len);

for i in 0..len {
ffi::PyList_SET_ITEM(ptr, i, decode_dag_cbor_to_pyobject(py, r, deep + 1)?.into_ptr());
ffi::PyList_SET_ITEM(ptr, i, decode_dag_cbor_to_pyobject(py, r, depth + 1)?.into_ptr());
}

let list: Bound<'_, PyList> = Bound::from_owned_ptr(py, ptr).downcast_into_unchecked();
Expand Down Expand Up @@ -162,8 +171,8 @@ fn decode_dag_cbor_to_pyobject<R: Read + Seek>(
let key_py = string_new_bound(py, key.as_slice()).to_object(py);
prev_key = Some(key);

let value = decode_dag_cbor_to_pyobject(py, r, deep + 1)?;
dict.set_item(key_py, value).unwrap();
let value_py = decode_dag_cbor_to_pyobject(py, r, depth + 1)?;
dict.set_item(key_py, value_py)?;
}

dict.to_object(py)
Expand All @@ -184,7 +193,7 @@ fn decode_dag_cbor_to_pyobject<R: Read + Seek>(
cbor::NULL => py.None(),
cbor::F32 => decode::read_f32(r)?.to_object(py),
cbor::F64 => decode::read_f64(r)?.to_object(py),
_ => return Err(anyhow!(format!("Unsupported major type"))),
_ => return Err(anyhow!("Unsupported major type".to_string())),
},
})
}
Expand Down Expand Up @@ -456,10 +465,20 @@ pub fn decode_dag_cbor(py: Python, data: &[u8]) -> PyResult<PyObject> {
if let Ok(py_object) = py_object {
Ok(py_object)
} else {
Err(get_err(
let err = get_err(
"Failed to decode DAG-CBOR",
py_object.unwrap_err().to_string(),
))
);

if let Some(py_err) = PyErr::take(py) {
py_err.set_cause(py, Option::from(err));
// in case something set global interpreter’s error,
// for example C FFI function, we should return it
// the real case: RecursionError (set by Py_EnterRecursiveCall)
Err(py_err)
} else {
Err(err)
}
}
}

Expand Down