Skip to content

fix[venom]: reject statically undersized abi_decode input#5094

Open
banteg wants to merge 4 commits into
vyperlang:masterfrom
banteg:fix/venom-abi-decode-size-check
Open

fix[venom]: reject statically undersized abi_decode input#5094
banteg wants to merge 4 commits into
vyperlang:masterfrom
banteg:fix/venom-abi-decode-size-check

Conversation

@banteg

@banteg banteg commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

What I did

Fixes #5055.

The Venom abi_decode lowering was missing legacy's compile-time check that the input bytestring's max length can fit the ABI size bound of the requested output type. abi_decode(data, (uint256, uint256)) with data: Bytes[4] compiled under --experimental-codegen into code that can only revert at runtime, while legacy rejects it at compile time.

How I did it

Ported the check from legacy ABIDecode.build_IR into lower_abi_decode, after wrapped_typ is computed and before any IR is emitted. The condition is the same as legacy: reject when input_max_len < wrapped_typ.abi_type.size_bound(), using StructureException at the input argument source location.

How to verify it

The shared length-mismatch table now includes the public abi_decode(data, (uint256, uint256)) shape. That row can pass on master in a default local run because it is exercising legacy codegen there; the differentiating regression is the explicit test_abi_decode_undersized_buffer_venom, which forces Settings(experimental_codegen=True).

I checked that the direct experimental-codegen reproducer compiles on origin/master and raises StructureException on this branch.

Focused validation:

uv run --python 3.12 -m pytest tests/functional/builtins/codegen/test_abi_decode.py -k "length_mismatch or undersized_buffer_venom" -q
uv run --python 3.12 -m pytest tests/functional/builtins/codegen/test_abi_decode.py -k "length_mismatch or undersized_buffer_venom" --experimental-codegen -q
uv run --python 3.12 black --check tests/functional/builtins/codegen/test_abi_decode.py vyper/codegen_venom/builtins/abi.py
git diff --check

Commit message

fix[venom]: reject statically undersized abi_decode input

The Venom abi_decode lowering only enforced runtime bounds checks and
was missing legacy's compile-time check that the input bytestring's
maxlen can fit the ABI size bound of the requested output type. Code
like `abi_decode(data, (uint256, uint256))` with `data: Bytes[4]`
compiled under experimental codegen into code that can only revert at
runtime, while legacy rejects it at compile time.

Port the check from ABIDecode.build_IR with the same condition,
exception type and source location.

Description for the changelog

Fix: experimental codegen rejects abi_decode with statically undersized input buffers at compile time, matching legacy.

Cute Animal Picture

cute animal

assert_compile_failed(lambda: get_contract(bad_code), exception)


def test_abi_decode_undersized_buffer_venom():

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we not already have a test for this which is just not tripped on venom pipeline?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there was already a length-mismatch compile-failure table. I folded the public abi_decode(data, (uint256, uint256)) undersized-buffer case into test_abi_decode_length_mismatch and removed the separate Venom-named test in b0de8d8. Validation: uv run --python 3.12 -m pytest tests/functional/builtins/codegen/test_abi_decode.py -k length_mismatch -q and the focused black check.

@charles-cooper charles-cooper left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok in principle; see my testing comment. paging @HodanPlodky for review

@github-actions

Copy link
Copy Markdown

Gas Changes

No changes detected.

Summary

  • Total tests measured: 815
  • Changed: 0
  • Regressions (gas up): 0
  • Improvements (gas down): 0
  • New tests: 0
  • Deleted tests: 0
  • Newly failing: 0
  • Newly passing: 0

@github-actions

Copy link
Copy Markdown

📊 Bytecode Size Changes (venom)

No changes detected.

Full bytecode sizes

Contract legacy-O2 legacy-Os -O2 -O3 -Os
curvefi/amm/stableswap/meta_implementation/meta_implementation_v_700.vy 23610 22805 20786 19755 19315
curvefi/legacy/CurveStableSwapMetaNG.vy 24952 23578 20490 19871 19175
curvefi/amm/stableswap/implementation/implementation_v_700.vy 24962 23769 20264 19393 18845
curvefi/legacy/CurveStableSwapNG.vy 24473 23298 19677 18844 18314
curvefi/amm/tricryptoswap/implementation/implementation_v_200.vy 20724 19959 18020 17518 16961
yearnfi/VaultV3.vy 19972 19063 17017 15131 14477
curvefi/amm/twocryptoswap/implementation/implementation_v_210.vy 17634 16894 15685 15132 14648
curvefi/legacy/CurveCryptoSwap2.vy 18947 18382 15634 15092 14711
yearnfi/VaultV2.vy 16676 15763 13863 13405 12574
curvefi/amm/stableswap/factory/factory_v_100.vy 14558 13978 13317 11825 11788
curvefi/gauge/child_gauge/implementation/implementation_v_110.vy 12338 11561 10270 9775 9182
curvefi/gauge/child_gauge/implementation/implementation_v_100.vy 12017 11249 9998 9502 8920
curvefi/amm/stableswap/views/views_v_120.vy 12784 12368 9970 9413 9532
curvefi/amm/tricryptoswap/math/math_v_200.vy 11189 11126 9520 8162 8424
curvefi/legacy/CurveCryptoMathOptimized3.vy 11188 11125 9519 8162 8424
curvefi/gauge/child_gauge/implementation/implementation_v_020.vy 10665 9947 9013 8565 8028
curvefi/helpers/router/router_v_110.vy 6717 6717 7003 6376 6607
curvefi/amm/tricryptoswap/views/views_v_200.vy 7821 7776 6697 6445 6514
curvefi/registries/metaregistry/metaregistry_v_110.vy 7590 6732 6651 5888 5727
curvefi/helpers/stable_swap_meta_zap/stable_swap_meta_zap_v_100.vy 7302 7067 6379 6012 6114
curvefi/amm/twocryptoswap/views/views_v_200.vy 6991 6946 6272 6026 6089
curvefi/amm/twocryptoswap/factory/factory_v_200.vy 5540 5252 5994 4697 4845
curvefi/amm/tricryptoswap/factory/factory_v_200.vy 5246 5021 5923 4926 5039
curvefi/registries/metaregistry/registry_handlers/stableswap/handler_v_110.vy 6633 6259 5859 5094 5562
curvefi/amm/twocryptoswap/math/math_v_210.vy 6800 6800 5581 5107 5112
curvefi/gauge/child_gauge/factory/factory_v_201.vy 4844 4547 4266 4014 3780
yearnfi/VaultFactory.vy 3765 3617 3985 2505 2965
curvefi/registries/metaregistry/registry_handlers/tricryptoswap/handler_v_110.vy 4241 3939 3838 3547 3547
curvefi/registries/metaregistry/registry_handlers/twocryptoswap/handler_v_110.vy 4186 3884 3777 3417 3433
curvefi/gauge/child_gauge/factory/factory_v_100.vy 4183 3914 3694 3428 3213
curvefi/helpers/rate_provider/rate_provider_v_101.vy 3260 3260 2779 2437 2450
curvefi/registries/address_provider/address_provider_v_201.vy 2973 2782 2713 2569 2424
curvefi/amm/stableswap/math/math_v_100.vy 3067 3046 2598 2403 2423
curvefi/helpers/rate_provider/rate_provider_v_100.vy 2847 2841 2357 2050 2058
curvefi/helpers/deposit_and_stake_zap/deposit_and_stake_zap_v_100.vy 2322 2316 2123 1889 1921
curvefi/governance/relayer/taiko/relayer_v_001.vy 2068 2064 1784 1579 1612
curvefi/governance/relayer/polygon_cdk/relayer_v_101.vy 1556 1523 1587 1405 1406
curvefi/governance/relayer/arb_orbit/relayer_v_101.vy 1266 1262 1280 1124 1155
curvefi/governance/relayer/op_stack/relayer_v_101.vy 1186 1182 1219 1066 1094
curvefi/governance/relayer/not_rollup/relayer_v_100.vy 1168 1153 1208 1061 1073
curvefi/governance/vault/vault_v_100.vy 964 941 885 866 864
curvefi/governance/relayer/relayer_v_100.vy 496 496 600 505 510
curvefi/governance/agent/agent_v_100.vy 541 541 443 415 419
curvefi/governance/agent/agent_v_101.vy 541 541 443 415 419

(
"""
@external
def foo(data: Bytes[4]) -> (uint256, uint256):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test passes also in the master why was the change in logic necessary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right - the folded table row passes on master in a default local run because that path uses legacy codegen. The regression is only in Venom.

I added test_abi_decode_undersized_buffer_venom, which forces Settings(experimental_codegen=True): on origin/master the reproducer compiles, while this branch raises StructureException. I also updated the PR body to make that distinction explicit.

Validation:

uv run --python 3.12 -m pytest tests/functional/builtins/codegen/test_abi_decode.py -k "length_mismatch or undersized_buffer_venom" -q
uv run --python 3.12 -m pytest tests/functional/builtins/codegen/test_abi_decode.py -k "length_mismatch or undersized_buffer_venom" --experimental-codegen -q
uv run --python 3.12 black --check tests/functional/builtins/codegen/test_abi_decode.py vyper/codegen_venom/builtins/abi.py
git diff --check

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.

Venom: abi_decode accepts statically undersized input buffers

3 participants