From 3c701654b4a067c19b723d019965275622aed033 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 15 Sep 2023 08:38:42 -0400 Subject: [PATCH] fix: only allow valid identifiers to be nonreentrant keys disallow invalid identifiers like `" "`, `"123abc"` from being keys for non-reentrant locks. this commit also refactors the `validate_identifiers` helper function to be in the `ast/` subdirectory, and slightly improves the VyperException constructor by allowing None (optional) annotations. --- .../exceptions/test_structure_exception.py | 23 +++- vyper/ast/identifiers.py | 111 ++++++++++++++++ vyper/exceptions.py | 4 +- vyper/semantics/namespace.py | 119 +----------------- vyper/semantics/types/base.py | 2 +- vyper/semantics/types/function.py | 6 +- 6 files changed, 143 insertions(+), 122 deletions(-) create mode 100644 vyper/ast/identifiers.py diff --git a/tests/parser/exceptions/test_structure_exception.py b/tests/parser/exceptions/test_structure_exception.py index 08794b75f2..97ac2b139d 100644 --- a/tests/parser/exceptions/test_structure_exception.py +++ b/tests/parser/exceptions/test_structure_exception.py @@ -56,9 +56,26 @@ def double_nonreentrant(): """, """ @external -@nonreentrant("B") -@nonreentrant("C") -def double_nonreentrant(): +@nonreentrant(" ") +def invalid_nonreentrant_key(): + pass + """, + """ +@external +@nonreentrant("") +def invalid_nonreentrant_key(): + pass + """, + """ +@external +@nonreentrant("123") +def invalid_nonreentrant_key(): + pass + """, + """ +@external +@nonreentrant("!123abcd") +def invalid_nonreentrant_key(): pass """, """ diff --git a/vyper/ast/identifiers.py b/vyper/ast/identifiers.py new file mode 100644 index 0000000000..985b04e5cd --- /dev/null +++ b/vyper/ast/identifiers.py @@ -0,0 +1,111 @@ +import re + +from vyper.exceptions import StructureException + + +def validate_identifier(attr, ast_node=None): + if not re.match("^[_a-zA-Z][a-zA-Z0-9_]*$", attr): + raise StructureException(f"'{attr}' contains invalid character(s)", ast_node) + if attr.lower() in RESERVED_KEYWORDS: + raise StructureException(f"'{attr}' is a reserved keyword", ast_node) + + +# https://docs.python.org/3/reference/lexical_analysis.html#keywords +# note we don't technically need to block all python reserved keywords, +# but do it for hygiene +_PYTHON_RESERVED_KEYWORDS = { + "False", + "None", + "True", + "and", + "as", + "assert", + "async", + "await", + "break", + "class", + "continue", + "def", + "del", + "elif", + "else", + "except", + "finally", + "for", + "from", + "global", + "if", + "import", + "in", + "is", + "lambda", + "nonlocal", + "not", + "or", + "pass", + "raise", + "return", + "try", + "while", + "with", + "yield", +} +_PYTHON_RESERVED_KEYWORDS = {s.lower() for s in _PYTHON_RESERVED_KEYWORDS} + +# Cannot be used for variable or member naming +RESERVED_KEYWORDS = _PYTHON_RESERVED_KEYWORDS | { + # decorators + "public", + "external", + "nonpayable", + "constant", + "immutable", + "transient", + "internal", + "payable", + "nonreentrant", + # "class" keywords + "interface", + "struct", + "event", + "enum", + # EVM operations + "unreachable", + # special functions (no name mangling) + "init", + "_init_", + "___init___", + "____init____", + "default", + "_default_", + "___default___", + "____default____", + # more control flow and special operations + "range", + # more special operations + "indexed", + # denominations + "ether", + "wei", + "finney", + "szabo", + "shannon", + "lovelace", + "ada", + "babbage", + "gwei", + "kwei", + "mwei", + "twei", + "pwei", + # sentinal constant values + # TODO remove when these are removed from the language + "zero_address", + "empty_bytes32", + "max_int128", + "min_int128", + "max_decimal", + "min_decimal", + "max_uint256", + "zero_wei", +} diff --git a/vyper/exceptions.py b/vyper/exceptions.py index aa23614e85..defca7cc53 100644 --- a/vyper/exceptions.py +++ b/vyper/exceptions.py @@ -54,7 +54,9 @@ def __init__(self, message="Error Message not found.", *items): # support older exceptions that don't annotate - remove this in the future! self.lineno, self.col_offset = items[0][:2] else: - self.annotations = items + # strip out None sources so that None can be passed as a valid + # annotation (in case it is only available optionally) + self.annotations = [k for k in items if k is not None] def with_annotation(self, *annotations): """ diff --git a/vyper/semantics/namespace.py b/vyper/semantics/namespace.py index b88bc3d817..613ac0c03b 100644 --- a/vyper/semantics/namespace.py +++ b/vyper/semantics/namespace.py @@ -1,12 +1,7 @@ import contextlib -import re - -from vyper.exceptions import ( - CompilerPanic, - NamespaceCollision, - StructureException, - UndeclaredDefinition, -) + +from vyper.ast.identifiers import validate_identifier +from vyper.exceptions import CompilerPanic, NamespaceCollision, UndeclaredDefinition from vyper.semantics.analysis.levenshtein_utils import get_levenshtein_error_suggestions @@ -121,111 +116,3 @@ def override_global_namespace(ns): finally: # unclobber _namespace = tmp - - -def validate_identifier(attr): - if not re.match("^[_a-zA-Z][a-zA-Z0-9_]*$", attr): - raise StructureException(f"'{attr}' contains invalid character(s)") - if attr.lower() in RESERVED_KEYWORDS: - raise StructureException(f"'{attr}' is a reserved keyword") - - -# https://docs.python.org/3/reference/lexical_analysis.html#keywords -# note we don't technically need to block all python reserved keywords, -# but do it for hygiene -_PYTHON_RESERVED_KEYWORDS = { - "False", - "None", - "True", - "and", - "as", - "assert", - "async", - "await", - "break", - "class", - "continue", - "def", - "del", - "elif", - "else", - "except", - "finally", - "for", - "from", - "global", - "if", - "import", - "in", - "is", - "lambda", - "nonlocal", - "not", - "or", - "pass", - "raise", - "return", - "try", - "while", - "with", - "yield", -} -_PYTHON_RESERVED_KEYWORDS = {s.lower() for s in _PYTHON_RESERVED_KEYWORDS} - -# Cannot be used for variable or member naming -RESERVED_KEYWORDS = _PYTHON_RESERVED_KEYWORDS | { - # decorators - "public", - "external", - "nonpayable", - "constant", - "immutable", - "transient", - "internal", - "payable", - "nonreentrant", - # "class" keywords - "interface", - "struct", - "event", - "enum", - # EVM operations - "unreachable", - # special functions (no name mangling) - "init", - "_init_", - "___init___", - "____init____", - "default", - "_default_", - "___default___", - "____default____", - # more control flow and special operations - "range", - # more special operations - "indexed", - # denominations - "ether", - "wei", - "finney", - "szabo", - "shannon", - "lovelace", - "ada", - "babbage", - "gwei", - "kwei", - "mwei", - "twei", - "pwei", - # sentinal constant values - # TODO remove when these are removed from the language - "zero_address", - "empty_bytes32", - "max_int128", - "min_int128", - "max_decimal", - "min_decimal", - "max_uint256", - "zero_wei", -} diff --git a/vyper/semantics/types/base.py b/vyper/semantics/types/base.py index af955f6071..c5af5c2a39 100644 --- a/vyper/semantics/types/base.py +++ b/vyper/semantics/types/base.py @@ -3,6 +3,7 @@ from vyper import ast as vy_ast from vyper.abi_types import ABIType +from vyper.ast.identifiers import validate_identifier from vyper.exceptions import ( CompilerPanic, InvalidLiteral, @@ -12,7 +13,6 @@ UnknownAttribute, ) from vyper.semantics.analysis.levenshtein_utils import get_levenshtein_error_suggestions -from vyper.semantics.namespace import validate_identifier # Some fake type with an overridden `compare_type` which accepts any RHS diff --git a/vyper/semantics/types/function.py b/vyper/semantics/types/function.py index 506dae135c..77b9efb13d 100644 --- a/vyper/semantics/types/function.py +++ b/vyper/semantics/types/function.py @@ -5,6 +5,7 @@ from typing import Any, Dict, List, Optional, Tuple from vyper import ast as vy_ast +from vyper.ast.identifiers import validate_identifier from vyper.ast.validation import validate_call_args from vyper.exceptions import ( ArgumentException, @@ -220,7 +221,10 @@ def from_FunctionDef( msg = "Nonreentrant decorator disallowed on `__init__`" raise FunctionDeclarationException(msg, decorator) - kwargs["nonreentrant"] = decorator.args[0].value + nonreentrant_key = decorator.args[0].value + validate_identifier(nonreentrant_key, decorator.args[0]) + + kwargs["nonreentrant"] = nonreentrant_key elif isinstance(decorator, vy_ast.Name): if FunctionVisibility.is_valid_value(decorator.id):