From 3adc4e3d9f9daf93ade89b7322a4ace85095c6e0 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 23 Mar 2024 07:53:26 -0400 Subject: [PATCH] feat[ci]: add conventional-commit validation to pull requests check single commit --- .github/workflows/pull-request.yaml | 54 ++++++++++++++++ setup.py | 5 +- .../functional/builtins/codegen/test_slice.py | 63 +++++++++++++++++++ .../codegen/modules/test_flag_imports.py | 41 ++++++++++++ vyper/ast/grammar.lark | 2 +- vyper/ast/parse.py | 2 +- vyper/builtins/functions.py | 22 +++++-- vyper/codegen/expr.py | 9 +-- vyper/semantics/analysis/module.py | 1 + vyper/semantics/types/module.py | 8 +++ 10 files changed, 189 insertions(+), 18 deletions(-) create mode 100644 .github/workflows/pull-request.yaml create mode 100644 tests/functional/codegen/modules/test_flag_imports.py diff --git a/.github/workflows/pull-request.yaml b/.github/workflows/pull-request.yaml new file mode 100644 index 00000000000..6e1138c6fe7 --- /dev/null +++ b/.github/workflows/pull-request.yaml @@ -0,0 +1,54 @@ +# jobs to validate pull request well-formedness + +name: Validate PR metadata + +on: + pull_request: + types: + - opened + - edited + - synchronize + +permissions: + pull-requests: read + +jobs: + conventional-commit: + name: Validate PR title + runs-on: ubuntu-latest + steps: + - uses: amannn/action-semantic-pull-request@v5 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # https://github.com/amannn/action-semantic-pull-request?tab=readme-ov-file#configuration + with: + types: | + feat + fix + chore + refactor + # ci: continuous integration + # docs: documentation + # test: test suite + # lang: language changes + # ux: language changes (UX) + # tool: integration + # ir: (old) IR/codegen changes + # venom: venom changes + scopes: | + ci + docs + test + lang + ux + tool + ir + venom + requireScope: true + subjectPattern: '^(?![A-Z]).+$' + subjectPatternError: | + Starts with uppercase letter: '{subject}' + (Full PR title: '{title}') + headerPattern: '^(\w*)(?:\[([\w$.\-*/ ]*)\])?: (.*)$' + validateSingleCommit: true + validateSingleCommitMatchesPrTitle: true diff --git a/setup.py b/setup.py index b48e1b7030e..a7943409fd5 100644 --- a/setup.py +++ b/setup.py @@ -31,13 +31,10 @@ "isort==5.13.2", "mypy==1.5", ], - "docs": ["recommonmark", "sphinx>=6.0,<7.0", "sphinx_rtd_theme>=1.2,<1.3"], "dev": ["ipython", "pre-commit", "pyinstaller", "twine"], } -extras_require["dev"] = ( - extras_require["test"] + extras_require["lint"] + extras_require["docs"] + extras_require["dev"] -) +extras_require["dev"] = extras_require["dev"] + extras_require["test"] + extras_require["lint"] with open("README.md", "r") as f: long_description = f.read() diff --git a/tests/functional/builtins/codegen/test_slice.py b/tests/functional/builtins/codegen/test_slice.py index 9fc464ed359..0ff6ee1d067 100644 --- a/tests/functional/builtins/codegen/test_slice.py +++ b/tests/functional/builtins/codegen/test_slice.py @@ -442,3 +442,66 @@ def test_slice_bytes32_calldata_extended(get_contract, code, result): c.bar(3, "0x0001020304050607080910111213141516171819202122232425262728293031", 5).hex() == result ) + + +# test cases crafted based on advisory GHSA-9x7f-gwxq-6f2c +oob_fail_list = [ + """ +d: public(Bytes[256]) + +@external +def do_slice(): + x : uint256 = max_value(uint256) + self.d = b"\x01\x02\x03\x04\x05\x06" + assert len(slice(self.d, 1, x)) == max_value(uint256) + """, + """ +@external +def do_slice(): + x: uint256 = max_value(uint256) + # y == 0x3232323232323232323232323232323232323232323232323232323232323232 + y: uint256 = 22704331223003175573249212746801550559464702875615796870481879217237868556850 + z: uint96 = 1 + if True: + placeholder : uint256[16] = [y, y, y, y, y, y, y, y, y, y, y, y, y, y, y, y] + s: String[32] = slice(uint2str(z), 1, x) + assert slice(s, 1, 2) == "22" + """, + """ +x: public(Bytes[64]) +secret: uint256 + +@deploy +def __init__(): + self.x = empty(Bytes[64]) + self.secret = 42 + +@external +def do_slice() -> Bytes[64]: + start: uint256 = max_value(uint256) - 63 + return slice(self.x, start, 64) + """, + # tests bounds check in adhoc location calldata + """ +interface IFace: + def choose_value(_x: uint256, _y: uint256, _z: uint256, idx: uint256) -> Bytes[32]: nonpayable + +@external +def choose_value(_x: uint256, _y: uint256, _z: uint256, idx: uint256) -> Bytes[32]: + assert idx % 32 == 4 + return slice(msg.data, idx, 32) + +@external +def do_slice(): + idx: uint256 = max_value(uint256) - 27 + ret: uint256 = _abi_decode(extcall IFace(self).choose_value(1, 2, 3, idx), uint256) + assert ret == 0 + """, +] + + +@pytest.mark.parametrize("bad_code", oob_fail_list) +def test_slice_buffer_oob_reverts(bad_code, get_contract, tx_failed): + c = get_contract(bad_code) + with tx_failed(): + c.do_slice() diff --git a/tests/functional/codegen/modules/test_flag_imports.py b/tests/functional/codegen/modules/test_flag_imports.py new file mode 100644 index 00000000000..fd954dab021 --- /dev/null +++ b/tests/functional/codegen/modules/test_flag_imports.py @@ -0,0 +1,41 @@ +def test_import_flag_types(make_input_bundle, get_contract): + lib1 = """ +import lib2 + +flag Roles: + ADMIN + USER + +enum Roles2: + ADMIN + USER + +role: Roles +role2: Roles2 +role3: lib2.Roles3 + """ + lib2 = """ +flag Roles3: + ADMIN + USER + NOBODY + """ + contract = """ +import lib1 + +initializes: lib1 + +@external +def bar(r: lib1.Roles, r2: lib1.Roles2, r3: lib1.lib2.Roles3) -> bool: + lib1.role = r + lib1.role2 = r2 + lib1.role3 = r3 + assert lib1.role == lib1.Roles.ADMIN + assert lib1.role2 == lib1.Roles2.USER + assert lib1.role3 == lib1.lib2.Roles3.NOBODY + return True + """ + + input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2}) + c = get_contract(contract, input_bundle=input_bundle) + assert c.bar(1, 2, 4) is True diff --git a/vyper/ast/grammar.lark b/vyper/ast/grammar.lark index cd7b64e6c3c..1c318a76a5d 100644 --- a/vyper/ast/grammar.lark +++ b/vyper/ast/grammar.lark @@ -98,7 +98,7 @@ tuple_def: "(" ( NAME | array_def | dyn_array_def | tuple_def ) ( "," ( NAME | a // NOTE: Map takes a basic type and maps to another type (can be non-basic, including maps) _MAP: "HashMap" map_def: _MAP "[" ( NAME | array_def ) "," type "]" -imported_type: NAME "." NAME +imported_type: NAME ("." NAME)+ type: ( NAME | imported_type | array_def | tuple_def | map_def | dyn_array_def ) // Structs can be composed of 1+ basic types or other custom_types diff --git a/vyper/ast/parse.py b/vyper/ast/parse.py index a4a86177304..690f88d3b5b 100644 --- a/vyper/ast/parse.py +++ b/vyper/ast/parse.py @@ -60,7 +60,7 @@ def parse_to_ast_with_settings( py_ast = python_ast.parse(python_source) except SyntaxError as e: # TODO: Ensure 1-to-1 match of source_code:reformatted_code SyntaxErrors - raise SyntaxException(str(e), vyper_source, e.lineno, e.offset) from e + raise SyntaxException(str(e), vyper_source, e.lineno, e.offset) from None # Add dummy function node to ensure local variables are treated as `AnnAssign` # instead of state variables (`VariableDecl`) diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py index c1ac244b4be..f29fd0ef618 100644 --- a/vyper/builtins/functions.py +++ b/vyper/builtins/functions.py @@ -229,6 +229,18 @@ def build_IR(self, expr, context): ADHOC_SLICE_NODE_MACROS = ["~calldata", "~selfcode", "~extcode"] +# make sure we don't overrun the source buffer, checking for overflow: +# valid inputs satisfy: +# `assert !(start+length > src_len || start+length < start` +def _make_slice_bounds_check(start, length, src_len): + with start.cache_when_complex("start") as (b1, start): + with add_ofst(start, length).cache_when_complex("end") as (b2, end): + arithmetic_overflow = ["lt", end, start] + buffer_oob = ["gt", end, src_len] + ok = ["iszero", ["or", arithmetic_overflow, buffer_oob]] + return b1.resolve(b2.resolve(["assert", ok])) + + def _build_adhoc_slice_node(sub: IRnode, start: IRnode, length: IRnode, context: Context) -> IRnode: assert length.is_literal, "typechecker failed" assert isinstance(length.value, int) # mypy hint @@ -241,7 +253,7 @@ def _build_adhoc_slice_node(sub: IRnode, start: IRnode, length: IRnode, context: if sub.value == "~calldata": node = [ "seq", - ["assert", ["le", ["add", start, length], "calldatasize"]], # runtime bounds check + _make_slice_bounds_check(start, length, "calldatasize"), ["mstore", np, length], ["calldatacopy", np + 32, start, length], np, @@ -251,7 +263,7 @@ def _build_adhoc_slice_node(sub: IRnode, start: IRnode, length: IRnode, context: elif sub.value == "~selfcode": node = [ "seq", - ["assert", ["le", ["add", start, length], "codesize"]], # runtime bounds check + _make_slice_bounds_check(start, length, "codesize"), ["mstore", np, length], ["codecopy", np + 32, start, length], np, @@ -266,8 +278,7 @@ def _build_adhoc_slice_node(sub: IRnode, start: IRnode, length: IRnode, context: sub.args[0], [ "seq", - # runtime bounds check - ["assert", ["le", ["add", start, length], ["extcodesize", "_extcode_address"]]], + _make_slice_bounds_check(start, length, ["extcodesize", "_extcode_address"]), ["mstore", np, length], ["extcodecopy", "_extcode_address", np + 32, start, length], np, @@ -440,8 +451,7 @@ def build_IR(self, expr, args, kwargs, context): ret = [ "seq", - # make sure we don't overrun the source buffer - ["assert", ["le", ["add", start, length], src_len]], # bounds check + _make_slice_bounds_check(start, length, src_len), do_copy, ["mstore", dst, length], # set length dst, # return pointer to dst diff --git a/vyper/codegen/expr.py b/vyper/codegen/expr.py index d7afe6c7f63..8ce4288c89c 100644 --- a/vyper/codegen/expr.py +++ b/vyper/codegen/expr.py @@ -206,12 +206,9 @@ def parse_Name(self): def parse_Attribute(self): typ = self.expr._metadata["type"] - # MyFlag.foo - if ( - isinstance(typ, FlagT) - and isinstance(self.expr.value, vy_ast.Name) - and typ.name == self.expr.value.id - ): + # check if we have a flag constant, e.g. + # [lib1].MyFlag.FOO + if isinstance(typ, FlagT) and is_type_t(self.expr.value._metadata["type"], FlagT): # 0, 1, 2, .. 255 flag_id = typ._flag_members[self.expr.attr] value = 2**flag_id # 0 => 0001, 1 => 0010, 2 => 0100, etc. diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index 90493d643bf..f4b7db129fe 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -657,6 +657,7 @@ def _validate_self_namespace(): def visit_FlagDef(self, node): obj = FlagT.from_FlagDef(node) + node._metadata["flag_type"] = obj self.namespace[node.name] = obj def visit_EventDef(self, node): diff --git a/vyper/semantics/types/module.py b/vyper/semantics/types/module.py index a242bfa1fe7..0d2b343e0d5 100644 --- a/vyper/semantics/types/module.py +++ b/vyper/semantics/types/module.py @@ -300,6 +300,10 @@ def __init__(self, module: vy_ast.Module, name: Optional[str] = None): # add the type of the event so it can be used in call position self.add_member(e.name, TYPE_T(e._metadata["event_type"])) # type: ignore + for f in self.flag_defs: + self.add_member(f.name, TYPE_T(f._metadata["flag_type"])) + self._helper.add_member(f.name, TYPE_T(f._metadata["flag_type"])) + for s in self.struct_defs: # add the type of the struct so it can be used in call position self.add_member(s.name, TYPE_T(s._metadata["struct_type"])) # type: ignore @@ -347,6 +351,10 @@ def function_defs(self): def event_defs(self): return self._module.get_children(vy_ast.EventDef) + @cached_property + def flag_defs(self): + return self._module.get_children(vy_ast.FlagDef) + @property def struct_defs(self): return self._module.get_children(vy_ast.StructDef)