From c5e57a86f0f3ce2fe12576a9c1a2edc783875d26 Mon Sep 17 00:00:00 2001 From: Vara Prasad Bandaru Date: Fri, 6 Jan 2023 22:55:45 +0530 Subject: [PATCH] Add tests with false positive patterns --- .github/workflows/pytest310.yml | 1 + tests/detectors/basic_router_application.py | 106 ++++ tests/detectors/false_positive_contracts.py | 597 ++++++++++++++++++++ tests/test_detectors.py | 4 + tests/test_detectors_using_pyteal.py | 49 ++ 5 files changed, 757 insertions(+) create mode 100644 tests/detectors/basic_router_application.py create mode 100644 tests/detectors/false_positive_contracts.py create mode 100644 tests/test_detectors_using_pyteal.py diff --git a/.github/workflows/pytest310.yml b/.github/workflows/pytest310.yml index c5ef6c8..9be69fa 100644 --- a/.github/workflows/pytest310.yml +++ b/.github/workflows/pytest310.yml @@ -54,6 +54,7 @@ jobs: - name: Run detectors tests in 3.10 run: | pytest tests/test_detectors.py + pytest tests/test_detectors_using_pyteal.py - name: Run dataflow analysis tests in 3.10 run: | diff --git a/tests/detectors/basic_router_application.py b/tests/detectors/basic_router_application.py new file mode 100644 index 0000000..e00b1bd --- /dev/null +++ b/tests/detectors/basic_router_application.py @@ -0,0 +1,106 @@ +# pylint: skip-file +# mypy: ignore-errors +from pyteal import * # pylint: disable=wildcard-import, unused-wildcard-import + +from pyteal import * +from typing import Literal + +from tealer.detectors.all_detectors import CanUpdate, CanDelete + +router = Router( + name="Example", + bare_calls=BareCallActions(), +) + + +@router.method(no_op=CallConfig.CALL) +def echo(input: abi.Uint64, *, output: abi.Uint64) -> Expr: + """ + Method config validations Teal pattern: + txn OnCompletion + int NoOp + == + txn ApplicationID + int 0 + != + && + assert // Assert(NoOp, CALL) + """ + return output.set(input.get()) + + +@router.method(no_op=CallConfig.CALL, opt_in=CallConfig.CREATE) +def deposit() -> Expr: + """ + Allow execution of this method if and only if: + - OnCompletion is NoOp AND `txn ApplicationID != 0`(Not a creation txn, so a Call) + Or + - OnCompletion is OptIn AND `txn ApplicationID == 0`(Creation Txn) + + Execution will fail for all other OnCompletion actions + + Method config validations Teal pattern: + txn OnCompletion + int NoOp + == + txn ApplicationID + int 0 + != + && // (NoOp && CALL) + txn OnCompletion + int OptIn + == + txn ApplicationID + int 0 + == + && // (OptIn && CREATE) + || + assert // Assert((NoOp && CALL) || (OptIn && CREATE)) + """ + return Return() + + +@router.method(no_op=CallConfig.ALL, opt_in=CallConfig.CALL, close_out=CallConfig.CALL) +def getBalance() -> Expr: + """ + Allow execution of this method if and only if: + - OnCompletion is NoOp; `txn ApplicationID` is not accessed/checked for CallConfig.ALL + Or + - OnCompletion is OptIn AND `txn ApplicationID == 0` (Creation Txn) + Or + - OnCompletion is CloseOut AND `txn ApplicationID != 0` (Not a creation txn -> Call) + + Method config validations Teal pattern: + txn OnCompletion + int NoOp + == // (NoOp && ALL) => (NoOp) + txn OnCompletion + int OptIn + == + txn ApplicationID + int 0 + != + && // (OptIn && CALL) + || + txn OnCompletion + int CloseOut + == + txn ApplicationID // Code is repeated for every CALL check; No optimizations + int 0 + != + && // (CloseOut && CALL) + || + assert // Assert((NoOp && ALL) || (OptIn && CALL) || (CloseOut && CALL)) + """ + return Return() + + +pragma(compiler_version="0.20.1") +basic_router_approval_program, clear_state_program, contract = router.compile_program( + version=7, # optimize=OptimizeOptions(scratch_slots=True) +) + +basic_router_tests = [ + (basic_router_approval_program, CanUpdate, []), + (basic_router_approval_program, CanDelete, []), +] diff --git a/tests/detectors/false_positive_contracts.py b/tests/detectors/false_positive_contracts.py new file mode 100644 index 0000000..81bd051 --- /dev/null +++ b/tests/detectors/false_positive_contracts.py @@ -0,0 +1,597 @@ +from typing import List, Tuple, Type + +from tealer.detectors.abstract_detector import AbstractDetector +from tealer.detectors.all_detectors import ( + CanCloseAccount, + MissingRekeyTo, + CanCloseAsset, + CanUpdate, + CanDelete, + MissingFeeCheck, +) + + +# fp for RekeyTo, AssetCloseTo, CloseRemainderTo +# reason: 1. && connected 2. index is checked and then fields of txn at that index +# reason 2 is solved +fp1_index_gtxn = """ +txn GroupIndex +int 1 +== +gtxn 1 AssetCloseTo +global ZeroAddress +== +&& +gtxn 1 CloseRemainderTo +global ZeroAddress +== +&& +gtxn 1 RekeyTo +global ZeroAddress +== +&& +assert +int 1 +return +""" + +FP1_TESTS: List[Tuple[str, Type[AbstractDetector], List[List[int]]]] = [ + (fp1_index_gtxn, MissingRekeyTo, []), + (fp1_index_gtxn, CanCloseAccount, []), + (fp1_index_gtxn, CanCloseAsset, []), +] + + +# fp for UpdateApplication, DeleteApplication +# reason: 1. && connected, 2. intcblock +fp2_and_connected = """ +#pragma version 4 +intcblock 0 +txn OnCompletion +intc_0 +== +txna ApplicationArgs 0 +pushbytes 0x5698b72d +== +&& +bnz main_l5 +err + +main_l5: +int 1 +return +""" + +FP2_TESTS: List[Tuple[str, Type[AbstractDetector], List[List[int]]]] = [ + (fp2_and_connected, CanUpdate, []), + (fp2_and_connected, CanDelete, []), +] + + +# fp for UpdateApplication, DeleteApplication +# reason: || (or) connected +fp3_or_connected = """ +#pragma version 4 +txn OnCompletion +int UpdateApplication +== +txn OnCompletion +int DeleteApplication +== +|| +txn OnCompletion +int CloseOut +== +|| +bnz fail + +success: +int 1 +return + +fail: +err +""" + +FP3_TESTS: List[Tuple[str, Type[AbstractDetector], List[List[int]]]] = [ + (fp3_or_connected, CanUpdate, []), + (fp3_or_connected, CanDelete, []), +] + + +# fp for DeleteApplication +# reason: `int 0` and `return` instructions are not sequential. +fp4_separate_return = """ +#pragma version 6 +txn OnCompletion +int DeleteApplication +== +bnz fail +int 1 +return +fail: +int 0 +b return_ins +return_ins: +return +""" + +FP4_TESTS: List[Tuple[str, Type[AbstractDetector], List[List[int]]]] = [ + (fp4_separate_return, CanDelete, []) +] + + +# fp for RekeyTo, Fee +# reason: && connected. +fp5_and_connected_with_branch = """ +#pragma version 4 +txn TypeEnum +int 2 +== +txn RekeyTo +global ZeroAddress +== +&& +txn LastValid +pushint 16600000 +== +&& +txn Fee +global MinTxnFee +<= +&& +txn Lease +pushbytes 0x0000000000000000000000000000000000000000000000000000000000000001 +== +&& +bz main_l2 +int 1 +return +main_l2: +txn RekeyTo +global ZeroAddress +== +txn CloseRemainderTo +global ZeroAddress +== +&& +txn Fee +int 2 +global MinTxnFee +* +<= +&& +assert +int 1 +return +""" + +FP5_TESTS: List[Tuple[str, Type[AbstractDetector], List[List[int]]]] = [ + (fp5_and_connected_with_branch, MissingRekeyTo, []), + (fp5_and_connected_with_branch, MissingFeeCheck, []), +] + + +# fp for Fee, CloseRemainderTo, AssetCloseTo detectors +# Reason: 1. && connected. 2. AssetCloseTo is only checked for axfer type transactions, 3. CloseRemainderTo is only checked for pay type transactions +fp6_multiple_group_sizes = """ +#pragma version 5 +global GroupSize +int 2 +== +bnz groupsize_2 +global GroupSize +int 3 +== +bnz groupsize_3 +global GroupSize +int 4 +== +gtxn 0 Fee +global MinTxnFee +== +&& +gtxn 1 Fee +global MinTxnFee +== +&& +gtxn 2 Fee +load 7 +== +&& +gtxn 3 Fee +int 0 +== +&& +gtxn 0 Sender +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 1 Sender +txn Sender +== +&& +gtxn 2 Sender +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 3 Sender +txn Sender +== +&& +gtxn 0 Amount +int 250000 +== +&& +gtxn 1 AssetAmount +int 0 +== +&& +gtxn 2 AssetAmount +int 1 +>= +&& +gtxn 3 Amount +int 0 +== +&& +gtxn 0 CloseRemainderTo +global ZeroAddress +== +&& +gtxn 1 AssetCloseTo +global ZeroAddress +== +&& +gtxn 2 AssetCloseTo +global ZeroAddress +== +&& +gtxn 3 CloseRemainderTo +global ZeroAddress +== +&& +gtxn 0 Receiver +txn Sender +== +&& +gtxn 1 AssetReceiver +txn Sender +== +&& +gtxn 2 AssetReceiver +txn Sender +== +&& +gtxn 3 Receiver +addr GDN6PPITDEXNCQ2BS2DUKVDPJZM7K6LKO6QBWP2US555NUE4Q5TY7HAVSQ +== +&& +gtxn 0 TypeEnum +int pay +== +&& +gtxn 1 TypeEnum +int axfer +== +&& +gtxn 2 TypeEnum +int axfer +== +&& +gtxn 3 TypeEnum +int pay +== +&& +gtxn 1 XferAsset +int 1337 +== +&& +gtxn 2 XferAsset +int 1337 +== +&& +assert +int 1 +return +groupsize_3: +global GroupSize +int 3 +== +gtxn 0 Fee +global MinTxnFee +== +&& +gtxn 1 Fee +global MinTxnFee +== +&& +gtxn 2 Fee +global MinTxnFee +== +&& +gtxn 0 Sender +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 1 Sender +txn Sender +== +&& +gtxn 2 Sender +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 0 Amount +int 250000 +== +&& +gtxn 1 AssetAmount +int 0 +== +&& +gtxn 2 AssetAmount +int 1 +>= +&& +gtxn 0 CloseRemainderTo +global ZeroAddress +== +&& +gtxn 1 AssetCloseTo +global ZeroAddress +== +&& +gtxn 2 AssetCloseTo +global ZeroAddress +== +&& +gtxn 0 Receiver +txn Sender +== +&& +gtxn 1 AssetReceiver +txn Sender +== +&& +gtxn 2 AssetReceiver +txn Sender +== +&& +gtxn 0 TypeEnum +int pay +== +&& +gtxn 1 TypeEnum +int axfer +== +&& +gtxn 2 TypeEnum +int axfer +== +&& +gtxn 1 XferAsset +int 1337 +== +&& +gtxn 2 XferAsset +int 1337 +== +&& +bz groupsize_3_2 +int 1 +return +groupsize_3_2: +global GroupSize +int 3 +== +gtxn 0 Fee +global MinTxnFee +== +&& +gtxn 1 Fee +global MinTxnFee +== +&& +gtxn 2 Fee +global MinTxnFee +== +&& +gtxn 0 Sender +txn Sender +== +&& +gtxn 1 Sender +txn Sender +== +&& +gtxn 2 Sender +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 0 AssetAmount +int 0 +== +&& +gtxn 1 Amount +int 0 +== +&& +gtxn 2 Amount +int 0 +== +&& +gtxn 0 AssetCloseTo +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 1 CloseRemainderTo +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 2 CloseRemainderTo +global ZeroAddress +== +&& +gtxn 0 AssetReceiver +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 1 Receiver +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 2 Receiver +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 0 TypeEnum +int axfer +== +&& +gtxn 1 TypeEnum +int pay +== +&& +gtxn 2 TypeEnum +int pay +== +&& +gtxn 0 XferAsset +int 1337 +== +&& +assert +int 1 +return +groupsize_2: +global GroupSize +int 2 +== +txn Fee +global MinTxnFee +== +&& +gtxn 0 Sender +txn Sender +== +&& +gtxn 1 Sender +txn Sender +== +&& +gtxn 0 AssetAmount +int 0 +== +&& +gtxn 1 Amount +int 50000 +== +&& +gtxn 0 AssetCloseTo +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 1 CloseRemainderTo +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 0 AssetReceiver +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 1 Receiver +addr BAZ7SJR2DVKCO6EHLLPXT7FRSYHNCZ35UTQD6K2FI4VALM2SSFIWTBZCTA +== +&& +gtxn 0 TypeEnum +int axfer +== +&& +gtxn 1 TypeEnum +int pay +== +&& +gtxn 0 XferAsset +int 1337 +== +&& +bz fail +int 1 +return +fail: +int 0 +return +""" + +FP6_TESTS: List[Tuple[str, Type[AbstractDetector], List[List[int]]]] = [ + (fp6_multiple_group_sizes, CanCloseAccount, []), + (fp6_multiple_group_sizes, CanCloseAsset, []), + (fp6_multiple_group_sizes, MissingFeeCheck, []), +] + + +# fp for AssetCloseTo +# reason: Only axfer type txns can have assetcloseto +fp7_txn_type_1 = """ +#pragma version 7 +txn TypeEnum +int pay +== +assert +int 1 +return +""" + +FP7_TESTS: List[Tuple[str, Type[AbstractDetector], List[List[int]]]] = [ + (fp7_txn_type_1, CanCloseAsset, []), +] + +# fp for CloseRemainderTo +# reason: Only pay type txns can have CloseRemainderTo +fp8_txn_type_2 = """ +#pragma version 7 +txn TypeEnum +int axfer +== +assert +int 1 +return +""" + +FP8_TESTS: List[Tuple[str, Type[AbstractDetector], List[List[int]]]] = [ + (fp8_txn_type_2, CanCloseAccount, []), +] + + +# fp for RekeyTo, CloseRemainderTo, AssetCloseTo, Fee +# reason: Applications are not vulnerable to RekeyTo, CloseRemainderTo, AssetCloseTo +fp9_txn_type_3 = """ +#pragma version 7 +txn OnCompletion +int OptIn +== +assert +int 1 +return +""" + +FP9_TESTS: List[Tuple[str, Type[AbstractDetector], List[List[int]]]] = [ + (fp9_txn_type_3, MissingRekeyTo, []), + (fp9_txn_type_3, CanCloseAccount, []), + (fp9_txn_type_3, CanCloseAsset, []), + (fp9_txn_type_3, MissingFeeCheck, []), +] + + +FP_TESTS: List[Tuple[str, Type[AbstractDetector], List[List[int]]]] = ( + FP1_TESTS + + FP2_TESTS + + FP3_TESTS + + FP4_TESTS + + FP5_TESTS + + FP6_TESTS + + FP7_TESTS + + FP8_TESTS + + FP9_TESTS +) diff --git a/tests/test_detectors.py b/tests/test_detectors.py index cc2eac6..15e8767 100644 --- a/tests/test_detectors.py +++ b/tests/test_detectors.py @@ -12,6 +12,7 @@ from tests.detectors.can_delete import can_delete_tests, new_can_delete_tests from tests.detectors.can_update import can_update_tests, new_can_update_tests from tests.detectors.rekeyto import missing_rekeyto_tests, new_missing_rekeyto_tests +from tests.detectors.false_positive_contracts import FP_TESTS from tests.utils import cmp_cfg @@ -46,6 +47,9 @@ def test_detectors(test: Tuple[str, Type[AbstractDetector], List[List[BasicBlock *new_can_delete_tests, ] +print("FP_TESTS:", len(FP_TESTS)) +ALL_NEW_TESTS += FP_TESTS + @pytest.mark.parametrize("test", ALL_NEW_TESTS) # type: ignore def test_just_detectors(test: Tuple[str, Type[AbstractDetector], List[List[int]]]) -> None: diff --git a/tests/test_detectors_using_pyteal.py b/tests/test_detectors_using_pyteal.py new file mode 100644 index 0000000..ed68988 --- /dev/null +++ b/tests/test_detectors_using_pyteal.py @@ -0,0 +1,49 @@ +# type: ignore[unreachable] +import sys +from typing import Tuple, List, Type + +import pytest + +from tealer.detectors.abstract_detector import AbstractDetector +from tealer.teal.parse_teal import parse_teal + +if not sys.version_info >= (3, 10): + pytest.skip(reason="PyTeal based tests require python >= 3.10", allow_module_level=True) + +# Place import statements after the version check +from tests.detectors.basic_router_application import ( # pylint: disable=wrong-import-position + basic_router_tests, +) + +TESTS: List[Tuple[str, Type[AbstractDetector], List[List[int]]]] = [ + *basic_router_tests, +] + + +@pytest.mark.parametrize("test", TESTS) # type: ignore +def test_parsing_using_pyteal(test: Tuple[str, Type[AbstractDetector], List[List[int]]]) -> None: + code, detector, expected_paths = test + teal = parse_teal(code.strip()) + teal.register_detector(detector) + result = teal.run_detectors()[0] + for bi in teal.bbs: + print( + bi, + bi.idx, + bi.transaction_context.transaction_types, + bi.transaction_context.group_indices, + ) + print( + bi.transaction_context.gtxn_context(0).transaction_types, + bi.transaction_context.group_indices, + ) + print( + bi.transaction_context.gtxn_context(1).transaction_types, + bi.transaction_context.group_indices, + ) + + assert len(result.paths) == len(expected_paths) + for path, expected_path in zip(result.paths, expected_paths): + assert len(path) == len(expected_path) + for bi, expected_idx in zip(path, expected_path): + assert bi.idx == expected_idx