Built-in shift() function will fail if passed a negative integer at compile time

[cyberthirst]

Submitted by obront.

Relevant GitHub Links

vyper/vyper/builtins/functions.py

Lines 1451 to 1466 in b01cd68

	def evaluate(self, node):
	if not self.__class__._warned:
	vyper_warn("`shift()` is deprecated! Please use the << or >> operator instead.")
	self.__class__._warned = True
	validate_call_args(node, 2)
	if [i for i in node.args if not isinstance(i, vy_ast.Int)]:
	raise UnfoldableNode
	value, shift = [i.value for i in node.args]
	if value < 0 or value >= 2**256:
	raise InvalidLiteral("Value out of range for uint256", node.args[0])
	if shift < -256 or shift > 256:
	# this validation is performed to prevent the compiler from hanging
	# rather than for correctness because the post-folded constant would
	# have been validated anyway
	raise InvalidLiteral("Shift must be between -256 and 256", node.args[1])

Summary

The built-in shift() function accepts an INT256 as an input, which are accounted for and work fine at runtime. However, there is a compile time check that causes a revert if a negative literal is passed to the function.

Vulnerability Details

In the evaluate() method, which is used when shift() is evaluated at compile time, there is the following check:

if value < 0 or value >= 2**256:
    raise InvalidLiteral("Value out of range for uint256", node.args[0])

However, the function is intended to accept INT256 as an argument:

_inputs = [("x", (UINT256_T, INT256_T)), ("_shift_bits", IntegerT.any())]

This is properly handled in the build_IR() method, but fails when evaluate() is called at compile time.

Impact

Contracts that shift a negative literal and attempt to evaluate the expression at compile time will fail to compile.

Tools Used

Manual Review

Recommendations

The ideal option would be to update the evaluate() method to handle negative integers.

Alternatively, given the shift() function is deprecated and may not justify the extra work, the easiest solution is to simply raise UnfoldableNote for values between type(int256).min and 0, which will skip evaluation and leave the function to be evaluated at runtime.