IS-1120: Prevalidate a TX more strictly

iconservice/icon_constant.py

@@ -139,7 +139,9 @@ class Revision(Enum):
 
     FIX_BALANCE_BUG = 11
 
-    LATEST = 11
+    IMPROVED_PRE_VALIDATOR = 12
+
+    LATEST = 12
 
 
 RC_DB_VERSION_0 = 0

iconservice/icon_inner_service.py

@@ -410,7 +410,7 @@ async def validate_transaction(self, request: dict):
     def _validate_transaction(self, request: dict):
         try:
             converted_request = TypeConverter.convert(request, ParamType.VALIDATE_TRANSACTION)
-            self._icon_service_engine.validate_transaction(converted_request)
+            self._icon_service_engine.validate_transaction(converted_request, request)
             response = MakeResponse.make_response(ExceptionCode.OK)
         except FatalException as e:
             self._log_exception(e, _TAG)

iconservice/icon_service_engine.py

@@ -33,7 +33,7 @@
 from .base.block import Block
 from .base.exception import (
     ExceptionCode, IconServiceBaseException, IconScoreException, InvalidBaseTransactionException,
-    InternalServiceErrorException, DatabaseException)
+    InternalServiceErrorException, DatabaseException, InvalidParamsException)
 from .base.message import Message
 from .base.transaction import Transaction
 from .base.type_converter_templates import ConstantKeys
@@ -1062,7 +1062,7 @@ def query(self, method: str, params: dict) -> Any:
         ret = self._call(context, method, params)
         return ret
 
-    def validate_transaction(self, request: dict) -> None:
+    def validate_transaction(self, request: dict, origin_request: dict) -> None:
         """Validate JSON-RPC transaction request
         before putting it into transaction pool
 
@@ -1073,6 +1073,7 @@ def validate_transaction(self, request: dict) -> None:
         :param request: JSON-RPC request
             values in request have already been converted to original format
             in IconInnerService
+        :param origin_request: JSON_RPC Original request for more strict validate
         :return:
         """
         assert self._get_context_stack_size() == 0
@@ -1100,11 +1101,17 @@ def validate_transaction(self, request: dict) -> None:
                 input_size = get_input_data_size(context.revision, data)
                 minimum_step += input_size * context.inv_container.step_costs.get(StepType.INPUT, 0)
 
+            self._icon_pre_validator.origin_request_execute(revision=context.revision, origin_request=origin_request)
             self._icon_pre_validator.execute(context, params, step_price, minimum_step)
 
-            # SCORE updating is not blocked by SCORE blacklist
-            if 'dataType' in params and params['dataType'] == 'call':
-                IconScoreContextUtil.validate_score_blacklist(context, to)
+            if "dataType" in params:
+                data_type: str = params["dataType"]
+                if data_type in ("call", "deploy", "deposit"):
+                    if to.is_contract:
+                        IconScoreContextUtil.validate_score_blacklist(context, to)
+                    else:
+                        if context.revision >= Revision.IMPROVED_PRE_VALIDATOR.value:
+                            raise InvalidParamsException(f"Invalid Contract Address: {to}")
         finally:
             self._pop_context()
 
@@ -1179,12 +1186,17 @@ def _handle_icx_call(cls,
         :return:
         """
 
-        icon_score_address: Address = params['to']
+        to: 'Address' = params['to']
         data_type = params.get('dataType', None)
         data = params.get('data', None)
 
         context.step_counter.apply_step(StepType.CONTRACT_CALL, 1)
-        return IconScoreEngine.query(context, icon_score_address, data_type, data)
+
+        if data_type == "call" and not to.is_contract:
+            if context.revision >= Revision.IMPROVED_PRE_VALIDATOR.value:
+                raise InvalidParamsException(f"Invalid Contract Address: {to}")
+
+        return IconScoreEngine.query(context, to, data_type, data)
 
     def _handle_icx_send_transaction(self,
                                      context: 'IconScoreContext',
@@ -1275,7 +1287,7 @@ def _process_transaction(self,
         input_size = get_input_data_size(context.revision, params.get('data', None))
         context.step_counter.apply_step(StepType.INPUT, input_size)
 
-        to: Address = params['to']
+        to: 'Address' = params['to']
         data_type: str = params.get('dataType')
 
         # Can't transfer ICX to system SCORE
@@ -1284,6 +1296,14 @@ def _process_transaction(self,
 
         if to.is_contract:
             tx_result.score_address = self._handle_score_invoke(context, to, params)
+        else:
+            if context.revision >= Revision.IMPROVED_PRE_VALIDATOR.value:
+                if data_type == "call":
+                    context.step_counter.apply_step(StepType.CONTRACT_CALL, 1)
+                    raise InvalidParamsException(f"Invalid Contract Address: {to}")
+                elif data_type == "deploy":
+                    context.step_counter.apply_step(StepType.CONTRACT_UPDATE, 1)
+                    raise InvalidParamsException(f"Invalid Contract Address: {to}")
 
     @classmethod
     def _transfer_coin(cls,

iconservice/iconscore/icon_pre_validator.py

@@ -19,8 +19,9 @@
 from iconcommons.logger import Logger
 
 from .icon_score_step import get_input_data_size
-from ..base.address import Address, SYSTEM_SCORE_ADDRESS, generate_score_address
+from ..base.address import Address, SYSTEM_SCORE_ADDRESS, generate_score_address, is_icon_address_valid
 from ..base.exception import InvalidRequestException, InvalidParamsException, OutOfBalanceException
+from ..base.type_converter_templates import ConstantKeys
 from ..icon_constant import FIXED_FEE, MAX_DATA_SIZE, DEFAULT_BYTE_SIZE, DATA_BYTE_ORDER, Revision, DeployState
 from ..utils import is_lowercase_hex_string
 from ..utils.locked import is_address_locked
@@ -44,6 +45,53 @@ def __init__(self) -> None:
         """
         pass
 
+    @classmethod
+    def origin_request_execute(cls, revision: int, origin_request: dict):
+        if not origin_request or revision < Revision.IMPROVED_PRE_VALIDATOR.value:
+            return
+
+        params: dict = origin_request[ConstantKeys.PARAMS]
+
+        if ConstantKeys.VERSION not in params:
+            raise InvalidRequestException(f"The version field is essential.")
+
+        version: str = params[ConstantKeys.VERSION]
+        if version == hex(2):
+            raise InvalidRequestException(f"Version2 is deprecated.")
+
+        invalid_v2: list = [
+            ConstantKeys.FEE,
+            ConstantKeys.OLD_TX_HASH
+        ]
+        for key in invalid_v2:
+            if key in params:
+                raise InvalidRequestException(f"Invalid v2 field: {key}")
+
+        int_list: list = [
+            ConstantKeys.VERSION,
+            ConstantKeys.VALUE,
+            ConstantKeys.STEP_LIMIT,
+            ConstantKeys.TIMESTAMP,
+            ConstantKeys.NONCE
+        ]
+        for key in int_list:
+            value: str = params[key]
+            try:
+                int_value: int = int(value, 16)
+                convert_hex: str = hex(int_value)
+                if value != convert_hex:
+                    raise InvalidRequestException(f"Malformed int: {value}")
+            except:
+                raise InvalidRequestException(f"Malformed int: {value}")
+
+        address_list: list = [
+            ConstantKeys.TO
+        ]
+        for key in address_list:
+            value: str = params[key]
+            if not is_icon_address_valid(value):
+                raise InvalidRequestException(f"Malformed Address: {value}")
+
     def execute(self, context: 'IconScoreContext', params: dict, step_price: int, minimum_step: int):
         """Validate a transaction on icx_sendTransaction
         If failed to validate a tx, raise an exception

tests/integrate_test/iiss/decentralized/test_integrate_test_call.py

@@ -0,0 +1,46 @@
+# -*- coding: utf-8 -*-
+# Copyright 2018 ICON Foundation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Test for icon_score_base.py and icon_score_base2.py"""
+from iconservice.icon_constant import ICX_IN_LOOP, Revision
+from tests.integrate_test.iiss.test_iiss_base import TestIISSBase
+
+
+class TestIntegrateCall(TestIISSBase):
+    def test_invalid_eoa_call(self):
+        self.init_decentralized()
+        self.init_inv()
+
+        balance: int = 10 * ICX_IN_LOOP
+        self.distribute_icx(
+            accounts=self._accounts[:1],
+            init_balance=balance
+        )
+
+        self.score_call(
+            from_=self._accounts[0],
+            to_=self._accounts[1].address,
+            func_name="test",
+        )
+
+        self.set_revision(Revision.IMPROVED_PRE_VALIDATOR.value)
+
+        self.score_call(
+            from_=self._accounts[0],
+            to_=self._accounts[1].address,
+            func_name="test",
+            pre_validation_enabled=False,
+            expected_status=False
+        )

tests/integrate_test/test_integrate_base.py

@@ -478,7 +478,7 @@ def create_deploy_score_tx(self,
         }
 
         if pre_validation_enabled:
-            self.icon_service_engine.validate_transaction(tx)
+            self.icon_service_engine.validate_transaction(tx, {})
 
         return tx
 
@@ -523,7 +523,7 @@ def create_score_call_tx(self,
         }
 
         if pre_validation_enabled:
-            self.icon_service_engine.validate_transaction(tx)
+            self.icon_service_engine.validate_transaction(tx, {})
 
         return tx
 
@@ -565,7 +565,7 @@ def create_transfer_icx_tx(self,
         }
 
         if not disable_pre_validate:
-            self.icon_service_engine.validate_transaction(tx)
+            self.icon_service_engine.validate_transaction(tx, {})
         return tx
 
     def create_message_tx(self,
@@ -602,7 +602,7 @@ def create_message_tx(self,
             'params': request_params
         }
 
-        self.icon_service_engine.validate_transaction(tx)
+        self.icon_service_engine.validate_transaction(tx, {})
         return tx
 
     def create_deposit_tx(self,
@@ -647,7 +647,7 @@ def create_deposit_tx(self,
         }
 
         if pre_validation_enabled:
-            self.icon_service_engine.validate_transaction(tx)
+            self.icon_service_engine.validate_transaction(tx, {})
 
         return tx
 
@@ -770,14 +770,17 @@ def score_call(self,
                    func_name: str,
                    params: dict = None,
                    value: int = 0,
+                   pre_validation_enabled: bool = True,
                    step_limit: int = DEFAULT_BIG_STEP_LIMIT,
+
                    expected_status: bool = True) -> List['TransactionResult']:
 
         tx = self.create_score_call_tx(from_=from_,
                                        to_=to_,
                                        func_name=func_name,
                                        params=params,
                                        value=value,
+                                       pre_validation_enabled=pre_validation_enabled,
                                        step_limit=step_limit)
         return self.process_confirm_block_tx([tx], expected_status)