Skip to content

Commit 0b74028

Browse files
charles-coopercharles-cooper
authored
fix: only allow valid identifiers to be nonreentrant keys (#3605)
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.
1 parent 344fd8f commit 0b74028

File tree

9 files changed

+147
-126
lines changed

9 files changed

+147
-126
lines changed

‎tests/parser/exceptions/test_structure_exception.py

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,26 @@ def double_nonreentrant():
5656
""",
5757
"""
5858
@external
59-
@nonreentrant("B")
60-
@nonreentrant("C")
61-
def double_nonreentrant():
59+
@nonreentrant(" ")
60+
def invalid_nonreentrant_key():
61+
pass
62+
""",
63+
"""
64+
@external
65+
@nonreentrant("")
66+
def invalid_nonreentrant_key():
67+
pass
68+
""",
69+
"""
70+
@external
71+
@nonreentrant("123")
72+
def invalid_nonreentrant_key():
73+
pass
74+
""",
75+
"""
76+
@external
77+
@nonreentrant("!123abcd")
78+
def invalid_nonreentrant_key():
6279
pass
6380
""",
6481
"""

‎tests/parser/features/decorators/test_nonreentrant.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ def set_callback(c: address):
142142
143143
@external
144144
@payable
145-
@nonreentrant('default')
145+
@nonreentrant("lock")
146146
def protected_function(val: String[100], do_callback: bool) -> uint256:
147147
self.special_value = val
148148
_amount: uint256 = msg.value
@@ -166,7 +166,7 @@ def unprotected_function(val: String[100], do_callback: bool):
166166
167167
@external
168168
@payable
169-
@nonreentrant('default')
169+
@nonreentrant("lock")
170170
def __default__():
171171
pass
172172
"""

‎tests/parser/test_call_graph_stability.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
from hypothesis import given, settings
77

88
import vyper.ast as vy_ast
9+
from vyper.ast.identifiers import RESERVED_KEYWORDS
910
from vyper.compiler.phases import CompilerData
10-
from vyper.semantics.namespace import RESERVED_KEYWORDS
1111

1212

1313
def _valid_identifier(attr):

‎tests/parser/types/test_identifier_naming.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import pytest
22

33
from vyper.ast.folding import BUILTIN_CONSTANTS
4+
from vyper.ast.identifiers import RESERVED_KEYWORDS
45
from vyper.builtins.functions import BUILTIN_FUNCTIONS
56
from vyper.codegen.expr import ENVIRONMENT_VARIABLES
67
from vyper.exceptions import NamespaceCollision, StructureException, SyntaxException
7-
from vyper.semantics.namespace import RESERVED_KEYWORDS
88
from vyper.semantics.types.primitives import AddressT
99

1010
BUILTIN_CONSTANTS = set(BUILTIN_CONSTANTS.keys())

‎vyper/ast/identifiers.py

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import re
2+
3+
from vyper.exceptions import StructureException
4+
5+
6+
def validate_identifier(attr, ast_node=None):
7+
if not re.match("^[_a-zA-Z][a-zA-Z0-9_]*$", attr):
8+
raise StructureException(f"'{attr}' contains invalid character(s)", ast_node)
9+
if attr.lower() in RESERVED_KEYWORDS:
10+
raise StructureException(f"'{attr}' is a reserved keyword", ast_node)
11+
12+
13+
# https://docs.python.org/3/reference/lexical_analysis.html#keywords
14+
# note we don't technically need to block all python reserved keywords,
15+
# but do it for hygiene
16+
_PYTHON_RESERVED_KEYWORDS = {
17+
"False",
18+
"None",
19+
"True",
20+
"and",
21+
"as",
22+
"assert",
23+
"async",
24+
"await",
25+
"break",
26+
"class",
27+
"continue",
28+
"def",
29+
"del",
30+
"elif",
31+
"else",
32+
"except",
33+
"finally",
34+
"for",
35+
"from",
36+
"global",
37+
"if",
38+
"import",
39+
"in",
40+
"is",
41+
"lambda",
42+
"nonlocal",
43+
"not",
44+
"or",
45+
"pass",
46+
"raise",
47+
"return",
48+
"try",
49+
"while",
50+
"with",
51+
"yield",
52+
}
53+
_PYTHON_RESERVED_KEYWORDS = {s.lower() for s in _PYTHON_RESERVED_KEYWORDS}
54+
55+
# Cannot be used for variable or member naming
56+
RESERVED_KEYWORDS = _PYTHON_RESERVED_KEYWORDS | {
57+
# decorators
58+
"public",
59+
"external",
60+
"nonpayable",
61+
"constant",
62+
"immutable",
63+
"transient",
64+
"internal",
65+
"payable",
66+
"nonreentrant",
67+
# "class" keywords
68+
"interface",
69+
"struct",
70+
"event",
71+
"enum",
72+
# EVM operations
73+
"unreachable",
74+
# special functions (no name mangling)
75+
"init",
76+
"_init_",
77+
"___init___",
78+
"____init____",
79+
"default",
80+
"_default_",
81+
"___default___",
82+
"____default____",
83+
# more control flow and special operations
84+
"range",
85+
# more special operations
86+
"indexed",
87+
# denominations
88+
"ether",
89+
"wei",
90+
"finney",
91+
"szabo",
92+
"shannon",
93+
"lovelace",
94+
"ada",
95+
"babbage",
96+
"gwei",
97+
"kwei",
98+
"mwei",
99+
"twei",
100+
"pwei",
101+
# sentinal constant values
102+
# TODO remove when these are removed from the language
103+
"zero_address",
104+
"empty_bytes32",
105+
"max_int128",
106+
"min_int128",
107+
"max_decimal",
108+
"min_decimal",
109+
"max_uint256",
110+
"zero_wei",
111+
}

‎vyper/exceptions.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@ def __init__(self, message="Error Message not found.", *items):
5454
# support older exceptions that don't annotate - remove this in the future!
5555
self.lineno, self.col_offset = items[0][:2]
5656
else:
57-
self.annotations = items
57+
# strip out None sources so that None can be passed as a valid
58+
# annotation (in case it is only available optionally)
59+
self.annotations = [k for k in items if k is not None]
5860

5961
def with_annotation(self, *annotations):
6062
"""

‎vyper/semantics/namespace.py

Lines changed: 3 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,7 @@
11
import contextlib
2-
import re
3-
4-
from vyper.exceptions import (
5-
CompilerPanic,
6-
NamespaceCollision,
7-
StructureException,
8-
UndeclaredDefinition,
9-
)
2+
3+
from vyper.ast.identifiers import validate_identifier
4+
from vyper.exceptions import CompilerPanic, NamespaceCollision, UndeclaredDefinition
105
from vyper.semantics.analysis.levenshtein_utils import get_levenshtein_error_suggestions
116

127

@@ -121,111 +116,3 @@ def override_global_namespace(ns):
121116
finally:
122117
# unclobber
123118
_namespace = tmp
124-
125-
126-
def validate_identifier(attr):
127-
if not re.match("^[_a-zA-Z][a-zA-Z0-9_]*$", attr):
128-
raise StructureException(f"'{attr}' contains invalid character(s)")
129-
if attr.lower() in RESERVED_KEYWORDS:
130-
raise StructureException(f"'{attr}' is a reserved keyword")
131-
132-
133-
# https://docs.python.org/3/reference/lexical_analysis.html#keywords
134-
# note we don't technically need to block all python reserved keywords,
135-
# but do it for hygiene
136-
_PYTHON_RESERVED_KEYWORDS = {
137-
"False",
138-
"None",
139-
"True",
140-
"and",
141-
"as",
142-
"assert",
143-
"async",
144-
"await",
145-
"break",
146-
"class",
147-
"continue",
148-
"def",
149-
"del",
150-
"elif",
151-
"else",
152-
"except",
153-
"finally",
154-
"for",
155-
"from",
156-
"global",
157-
"if",
158-
"import",
159-
"in",
160-
"is",
161-
"lambda",
162-
"nonlocal",
163-
"not",
164-
"or",
165-
"pass",
166-
"raise",
167-
"return",
168-
"try",
169-
"while",
170-
"with",
171-
"yield",
172-
}
173-
_PYTHON_RESERVED_KEYWORDS = {s.lower() for s in _PYTHON_RESERVED_KEYWORDS}
174-
175-
# Cannot be used for variable or member naming
176-
RESERVED_KEYWORDS = _PYTHON_RESERVED_KEYWORDS | {
177-
# decorators
178-
"public",
179-
"external",
180-
"nonpayable",
181-
"constant",
182-
"immutable",
183-
"transient",
184-
"internal",
185-
"payable",
186-
"nonreentrant",
187-
# "class" keywords
188-
"interface",
189-
"struct",
190-
"event",
191-
"enum",
192-
# EVM operations
193-
"unreachable",
194-
# special functions (no name mangling)
195-
"init",
196-
"_init_",
197-
"___init___",
198-
"____init____",
199-
"default",
200-
"_default_",
201-
"___default___",
202-
"____default____",
203-
# more control flow and special operations
204-
"range",
205-
# more special operations
206-
"indexed",
207-
# denominations
208-
"ether",
209-
"wei",
210-
"finney",
211-
"szabo",
212-
"shannon",
213-
"lovelace",
214-
"ada",
215-
"babbage",
216-
"gwei",
217-
"kwei",
218-
"mwei",
219-
"twei",
220-
"pwei",
221-
# sentinal constant values
222-
# TODO remove when these are removed from the language
223-
"zero_address",
224-
"empty_bytes32",
225-
"max_int128",
226-
"min_int128",
227-
"max_decimal",
228-
"min_decimal",
229-
"max_uint256",
230-
"zero_wei",
231-
}

‎vyper/semantics/types/base.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
from vyper import ast as vy_ast
55
from vyper.abi_types import ABIType
6+
from vyper.ast.identifiers import validate_identifier
67
from vyper.exceptions import (
78
CompilerPanic,
89
InvalidLiteral,
@@ -12,7 +13,6 @@
1213
UnknownAttribute,
1314
)
1415
from vyper.semantics.analysis.levenshtein_utils import get_levenshtein_error_suggestions
15-
from vyper.semantics.namespace import validate_identifier
1616

1717

1818
# Some fake type with an overridden `compare_type` which accepts any RHS

‎vyper/semantics/types/function.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from typing import Any, Dict, List, Optional, Tuple
66

77
from vyper import ast as vy_ast
8+
from vyper.ast.identifiers import validate_identifier
89
from vyper.ast.validation import validate_call_args
910
from vyper.exceptions import (
1011
ArgumentException,
@@ -220,7 +221,10 @@ def from_FunctionDef(
220221
msg = "Nonreentrant decorator disallowed on `__init__`"
221222
raise FunctionDeclarationException(msg, decorator)
222223

223-
kwargs["nonreentrant"] = decorator.args[0].value
224+
nonreentrant_key = decorator.args[0].value
225+
validate_identifier(nonreentrant_key, decorator.args[0])
226+
227+
kwargs["nonreentrant"] = nonreentrant_key
224228

225229
elif isinstance(decorator, vy_ast.Name):
226230
if FunctionVisibility.is_valid_value(decorator.id):

0 commit comments

Comments
 (0)