Skip to content

fix(types): reject odd-length hex strings - #100

Open
UgwujaGeorge wants to merge 1 commit into
tempoxyz:mainfrom
UgwujaGeorge:fix/reject-odd-length-hex
Open

UgwujaGeorge wants to merge 1 commit into
tempoxyz:mainfrom
UgwujaGeorge:fix/reject-odd-length-hex

Conversation

@UgwujaGeorge

Copy link
Copy Markdown

Summary

as_bytes, as_address and as_hash32 pass hex strings straight to eth_utils.to_bytes, which left-pads an odd-length string with a zero nibble. The byte-length check that follows then passes, because the padding has already produced the expected length:

b = to_bytes(hexstr=value)          # "0x" + "a" * 39  ->  20 bytes
if len(b) not in (0, 20):           # passes
    raise ValueError(...)

So an address that lost a single character is accepted as a valid 20-byte address — a different one, with every byte shifted:

>>> Call.create(to="0x70997970C51812dc3a010C7d01b50e0d17dc79C8").to.hex()  # intended
'70997970c51812dc3a010c7d01b50e0d17dc79c8'
>>> Call.create(to="0x70997970C51812dc3a010C7d01b50e0d17dc79C").to.hex()   # one char dropped
'070997970c51812dc3a010c7d01b50e0d17dc79c'

No error is raised at any point. The same shift applies to as_hash32 (63 digits) and as_selector (7 digits), and reaches Call.to, AccessListItem.address, its storage_keys, and the access-key call scopes in keychain.py, all of which use these as attrs converters. The helpers are also exported from pytempo, so callers use them directly.

This is the same class as #90 — a malformed value silently reinterpreted rather than rejected — except here the corruption happens at input coercion rather than at decode.

Fix

Reject an odd number of hex digits in _hex_to_bytes, shared by the three string branches. as_selector already routes through as_bytes, so it is covered too. Even-length input decodes exactly as before.

Tests

Adds six cases to TestTypes: odd-length rejected for as_address, as_hash32, as_bytes and as_selector, rejected with the 0x prefix omitted, and a guard that canonical even-length input still decodes unchanged.

The five rejection tests fail on main and pass on this branch. Full suite 287 passed, 37 skipped (+6); ruff check . and ruff format --check . clean.

eth_utils.to_bytes left-pads an odd-length hex string with a zero nibble, so
an address that lost a character decoded to 20 bytes and passed the length
check as a different, well-formed address. as_bytes, as_address, as_hash32 and
as_selector now reject odd-length hex instead, and add regression tests.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant