Skip to content

Commit

Permalink
Fix stack overflow when decoding deeply nested structures like arrays…
Browse files Browse the repository at this point in the history
… and maps (#51)
  • Loading branch information
MarshalX authored Feb 17, 2025
1 parent 7bf9fb9 commit 744bc2f
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 15 deletions.
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

0 comments on commit 744bc2f

Please sign in to comment.