vyperlang / vyper

Pythonic Smart Contract Language for the EVM
https://vyperlang.org
Other
4.87k stars 796 forks source link

_bytes_to_num() skips ensure_in_memory() check, which can lead to compilation failure #4136

Open cyberthirst opened 6 months ago

cyberthirst commented 6 months ago

Submitted by obront.

Relevant GitHub Links

https://github.com/vyperlang/vyper/blob/b01cd686aa567b32498fefd76bd96b0597c6f099/vyper/builtins/_convert.py#L76-L85

Summary

The _bytes_to_num() function used in conversion assumes that any bytestring types are in memory. If they are declared from within the expression, it will try to load them from memory and panic.

Vulnerability Details

When converting a bytestring to a number, we perform the following:

if isinstance(arg.typ, _BytestringT):
    _len = get_bytearray_length(arg)
    arg = LOAD(bytes_data_ptr(arg))
    num_zero_bits = ["mul", 8, ["sub", 32, _len]]

The get_bytearray_length() function correctly handles the case when an empty bytestring is passed directly to the conversion. However, the bytes_data_ptr() function panics if the argument doesn't have a location specified:

if ptr.location is None:
    raise CompilerPanic("tried to modify non-pointer type")

Proof of Concept

The following Vyper contract should compile:

@external
def get_empty_bytestring_as_uint() -> uint256:
    return convert(empty(Bytes[32]), uint256)

However, it instead returns the following:

Error compiling: examples/minimal.vy
vyper.exceptions.CompilerPanic: tried to modify non-pointer type

Impact

Contracts that include a conversion where an empty bytestring is declared within the expression will fail to compile.

Tools Used

Manual Review

Recommendations

Use something like the ensure_in_memory() function used elsewhere in the compiler to move empty bytestrings to memory before converting them, or create a manual override for empty strings to return the appropriate value.