Skip to content

Commit

Permalink
add check for pointer validity during decoding plus tests
Browse files Browse the repository at this point in the history
  • Loading branch information
pacrob committed Feb 16, 2024
1 parent bf9cb50 commit 24afe35
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 5 deletions.
45 changes: 41 additions & 4 deletions eth_abi/decoding.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ def __init__(self, *args, **kwargs):
self._frames = []
self._total_offset = 0

def seek_in_frame(self, pos, *args, **kwargs):
def seek_in_frame(self, pos: int, *args: Any, **kwargs: Any) -> None:
"""
Seeks relative to the total offset of the current contextual frames.
"""
self.seek(self._total_offset + pos, *args, **kwargs)

def push_frame(self, offset):
def push_frame(self, offset: int) -> None:
"""
Pushes a new contextual frame onto the stack with the given offset and a
return position at the current cursor position then seeks to the new
Expand Down Expand Up @@ -131,6 +131,20 @@ def __call__(self, stream: ContextFramesBytesIO) -> Any:


class HeadTailDecoder(BaseDecoder):
"""
Decoder for the contents of a dynamic container. Each dynamic container has 2 main
parts: the first 32 bytes * length of the container contains the offsets to
the start of each element in the container, and the second part contains the actual
elements.
A dynamic container is a either a dynamic array or a tuple with dynamic elements.
This decoder will be called once for each element in the container, reading the
offset, jumping to the location of the data, decoding the data, and then jumping
back to the original location. The parent container will then be responsible for
moving the stream to the next offset and calling this decoder again.
"""

is_dynamic = True

tail_decoder = None
Expand All @@ -141,13 +155,19 @@ def validate(self):
if self.tail_decoder is None:
raise ValueError("No `tail_decoder` set")

def decode(self, stream):
def decode(self, stream: ContextFramesBytesIO) -> Any:
# Decode the head section, which gives how many bytes to jump ahead in the
# stream to find to the tail section
start_pos = decode_uint_256(stream)

# Jump ahead in the stream to the tail section
stream.push_frame(start_pos)

# assertion check for mypy
if self.tail_decoder is None:
raise AssertionError("`tail_decoder` is None")
# Decode the tail section
value = self.tail_decoder(stream)
# Return to the original position in the stream
stream.pop_frame()

return value
Expand Down Expand Up @@ -275,6 +295,19 @@ def decode(self, stream):
stream.push_frame(32)
if self.item_decoder is None:
raise AssertionError("`item_decoder` is None")

# verify nested dynamic arrays have valid pointers
if isinstance(self.item_decoder, HeadTailDecoder):
current_location = stream.tell()
end_of_offsets = current_location + 32 * array_size
for _ in range(array_size):
offset = decode_uint_256(stream)
if offset + current_location < end_of_offsets:
# the pointer is indicating its data is located within the offsets
# section of the stream, which is invalid
raise ValueError("Invalid pointer")
stream.seek(current_location)

for _ in range(array_size):
yield self.item_decoder(stream)
stream.pop_frame()
Expand Down Expand Up @@ -515,6 +548,10 @@ def decoder_fn(data):
return data

def read_data_from_stream(self, stream):
# The first 32 bytes from the stream's cursor give the length of the data,
# i.e. how many more bytes to read from the stream to get the value.

# Read length and move cursor forward 32 bytes.
data_length = decode_uint_256(stream)
padded_length = ceil32(data_length)

Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
],
"test": [
"pytest>=7.0.0",
"pytest-timeout>=2.0.0",
"pytest-xdist>=2.4.0",
"pytest-pythonpath>=0.7.1",
"eth-hash[pycryptodome]",
Expand Down
28 changes: 27 additions & 1 deletion tests/abi/test_decode.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ def test_abi_decode_for_single_dynamic_types(
)

(actual,) = decode([type_str], abi_encoding, strict=strict)

assert actual == expected


Expand Down Expand Up @@ -198,3 +197,30 @@ def test_abi_decode_with_shorter_data_than_32_bytes(types, hex_data, expected):
# without the flag set (i.e. assert the default behavior is always ``strict=True``).
with pytest.raises(InsufficientDataBytes):
decode(types, bytes.fromhex(hex_data))


@pytest.mark.parametrize(
"typestring",
(
"uint256[][][][][][][][][][]",
"uint256[][][][][][][][][][][][]",
"(uint256[][][][][][][][][][][][])",
"((bytes[]))",
),
)
@pytest.mark.parametrize(
"payload",
(
("0" * 62 + "20") * 10 + "00" * 2056,
"0" * 62 + "20" + "0" * 62 + "a0" + ("0" * 62 + "20") * 9 + "00" * 1024,
),
ids=(
"all pointers equal equal to 0x20",
"user example",
),
)
@pytest.mark.timeout(1)
def test_decode_nested_array_with_invalid_pointer_fails_fast(typestring, payload):
malformed_payload_bytes = bytearray.fromhex(payload)
with pytest.raises(ValueError, match="Invalid pointer"):
decode([typestring], malformed_payload_bytes)

0 comments on commit 24afe35

Please sign in to comment.