Tuple constants are deleted during folding, breaking compilation

Severity

Medium Risk

Relevant GitHub Links

https://github.com/vyperlang/vyper/blob/master/vyper/ast/folding.py

https://github.com/vyperlang/vyper/blob/b01cd686aa567b32498fefd76bd96b0597c6f099/vyper/ast/expansion.py#L97-L113

Summary

During constant folding, references to constant variables are replaced by their underlying values. After this is done, the constant variable itself is deleted. In the case of tuple constants, the first step fails. This results in references to non-existent variables, which breaks the compilation process later on, in the codegen module.

Vulnerability Details

In the replace_user_defined_constants() function within the folding process, we go through all constant variables and iterate through all references to that value within the source code.

for node in vyper_module.get_descendants(vy_ast.Name, {"id": id_}, reverse=True):
    ...

For each instance, we call _replace(), which attempts to create a new node with the values from the old node, but the type changed to the type of the constant and the value set to the constant's value. This call is wrapped in a try catch block, so that UnfoldableNode errors do not break the compilation, but instead simply remain to be returned at runtime.

try:
    new_node = _replace(node, replacement_node, type_=type_)
except UnfoldableNode:
    if raise_on_error:
        raise
    continue

If we look at the _replace() function, we can see that it handles when the value is a Constant, a List, or a Call, but returns UnfoldableNote in all other cases.

Comparing this to the checks within the semantic analysis, we can see that semantically we allow tuples to be constants, while the the folding process this will skip the folding due to an error:

def check_constant(node: vy_ast.VyperNode) -> bool:
    """
    Check if the given node is a literal or constant value.
    """
    if _check_literal(node):
        return True
    if isinstance(node, (vy_ast.Tuple, vy_ast.List)):
        return all(check_constant(item) for item in node.elements)
    if isinstance(node, vy_ast.Call):
        args = node.args
        if len(args) == 1 and isinstance(args[0], vy_ast.Dict):
            return all(check_constant(v) for v in args[0].values)

        call_type = get_exact_type_from_node(node.func)
        if getattr(call_type, "_kwargable", False):
            return True

    return False

After the folding is complete, the remove_unused_statements() function removes all nodes that represent variable declarations to constants. This assumes that these will all have been replaced in-place where they are used, but does not take into account that tuples have been skipped.

def remove_unused_statements(vyper_module: vy_ast.Module) -> None:
"""
Remove statement nodes that are unused after type checking.

Once type checking is complete, we can remove now-meaningless statements to
simplify the AST prior to IR generation.

Arguments
---------
vyper_module : Module
    Top-level Vyper AST node.
"""

for node in vyper_module.get_children(vy_ast.VariableDecl, {"is_constant": True}):
    vyper_module.remove_from_body(node)

# `implements: interface` statements - validated during type checking
for node in vyper_module.get_children(vy_ast.ImplementsDecl):
    vyper_module.remove_from_body(node)

The result is that any tuple constants are NOT replaced in-place during folding, but DO have their nodes deleted after folding is complete. This leads to an error further along the pipeline where the codegen module tries to parse_Name and finds that the corresponding variable name does not exist.

Proof of Concept

# @version ^0.3.9

e: constant(uint256) = 24
f: constant((uint256, uint256)) = (e, e)

@external
def foo(x: uint256) -> uint256:
    return f[0]

This results in the following error:

vyper.exceptions.TypeCheckFailure: Name node did not produce IR.

Impact

Tuple constants that are declared will not be properly handled and instead will cause compilation to fail.

Note that while I have not been able to identify any way to have the code compile despite the missed checks, if there were any edge cases where these tuple values could be used within the contract without reverting the compilation, issues could creep into compiled code that could have more serious implications.

Tools Used

Manual Review

Recommendations

Adjust the _replace() function to handle tuples properly, or explicitly disallow them from being used as constants to catch this situation in the semantic analysis.