From 37b4133558646bc75679019f1ef592a5aa41993b Mon Sep 17 00:00:00 2001
From: Mathieu <60658558+enitrat@users.noreply.github.com>
Date: Tue, 13 Feb 2024 11:46:18 +0100
Subject: [PATCH] fix: returndatacopy error when out of bounds (#975)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!--- Please provide a general summary of your changes in the title
above -->

<!-- Give an estimate of the time you spent on this PR in terms of work
days.
Did you spend 0.5 days on this PR or rather 2 days?  -->

Time spent on this PR: 0.2

## Pull request type

<!-- Please try to limit your pull request to one type,
submit multiple pull requests if needed. -->

Please check the type of change your PR introduces:

- [ ] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):

## What is the current behavior?

<!-- Please describe the current behavior that you are modifying,
or link to a relevant issue. -->

Resolves #<Issue number>

## What is the new behavior?

<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Fixes returndatacopy that was able to read out of bounds
-
-

<!-- Reviewable:start -->
- - -
This change is [<img src="https://reviewable.io/review_button.svg"
height="34" align="absmiddle"
alt="Reviewable"/>](https://reviewable.io/reviews/kkrt-labs/kakarot/975)
<!-- Reviewable:end -->
---
 blockchain-tests-skip.yml                     | 14 ---------
 src/kakarot/errors.cairo                      | 31 +++++++++++++++++++
 .../environmental_information.cairo           | 15 ++++++++-
 3 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/blockchain-tests-skip.yml b/blockchain-tests-skip.yml
index 8375a6bf4..fc49d8e3a 100644
--- a/blockchain-tests-skip.yml
+++ b/blockchain-tests-skip.yml
@@ -433,20 +433,6 @@ testname:
     - returndatacopy_initial_d0g0v0_Shanghai
     - returndatacopy_overrun_d0g0v0_Shanghai
     - subcallReturnMoreThenExpected_d0g0v0_Shanghai
-    - tooLongReturnDataCopy_d10g0v0_Shanghai
-    - tooLongReturnDataCopy_d11g0v0_Shanghai
-    - tooLongReturnDataCopy_d13g0v0_Shanghai
-    - tooLongReturnDataCopy_d16g0v0_Shanghai
-    - tooLongReturnDataCopy_d18g0v0_Shanghai
-    - tooLongReturnDataCopy_d1g0v0_Shanghai
-    - tooLongReturnDataCopy_d20g0v0_Shanghai
-    - tooLongReturnDataCopy_d21g0v0_Shanghai
-    - tooLongReturnDataCopy_d22g0v0_Shanghai
-    - tooLongReturnDataCopy_d23g0v0_Shanghai
-    - tooLongReturnDataCopy_d4g0v0_Shanghai
-    - tooLongReturnDataCopy_d6g0v0_Shanghai
-    - tooLongReturnDataCopy_d8g0v0_Shanghai
-    - tooLongReturnDataCopy_d9g0v0_Shanghai
   stRevertTest:
     - RevertInCallCode_d0g0v0_Shanghai
     - RevertInDelegateCall_d0g0v0_Shanghai
diff --git a/src/kakarot/errors.cairo b/src/kakarot/errors.cairo
index fad12045e..ad21d4b5b 100644
--- a/src/kakarot/errors.cairo
+++ b/src/kakarot/errors.cairo
@@ -326,6 +326,37 @@ namespace Errors {
         dw 119;  // w
     }
 
+    func outOfBoundsRead() -> (error_len: felt, error: felt*) {
+        let (error) = get_label_location(out_of_bounds_read_error_message);
+        return (24, error);
+
+        out_of_bounds_read_error_message:
+        dw 'K';
+        dw 'a';
+        dw 'k';
+        dw 'a';
+        dw 'r';
+        dw 'o';
+        dw 't';
+        dw ':';
+        dw ' ';
+        dw 'O';
+        dw 'u';
+        dw 't';
+        dw 'O';
+        dw 'f';
+        dw 'B';
+        dw 'o';
+        dw 'u';
+        dw 'n';
+        dw 'd';
+        dw 's';
+        dw 'R';
+        dw 'e';
+        dw 'a';
+        dw 'd';
+    }
+
     func unknownPrecompile(address: felt) -> (error_len: felt, error: felt*) {
         alloc_locals;
         let (error) = alloc();
diff --git a/src/kakarot/instructions/environmental_information.cairo b/src/kakarot/instructions/environmental_information.cairo
index 599c69f9f..d14a030ea 100644
--- a/src/kakarot/instructions/environmental_information.cairo
+++ b/src/kakarot/instructions/environmental_information.cairo
@@ -8,10 +8,11 @@ from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin
 from starkware.cairo.common.cairo_keccak.keccak import cairo_keccak_bigend, finalize_keccak
 from starkware.cairo.common.memset import memset
 from starkware.cairo.common.math import unsigned_div_rem
-from starkware.cairo.common.math_cmp import is_not_zero
+from starkware.cairo.common.math_cmp import is_not_zero, is_le
 
 from kakarot.account import Account
 from kakarot.evm import EVM
+from kakarot.errors import Errors
 from kakarot.gas import Gas
 from kakarot.memory import Memory
 from kakarot.model import model
@@ -205,15 +206,27 @@ namespace EnvironmentalInformation {
         if (opcode_number == 0x37) {
             assert data_len = evm.message.calldata_len;
             assert data = evm.message.calldata;
+            tempvar range_check_ptr = range_check_ptr;
         } else {
             if (opcode_number == 0x39) {
                 assert data_len = evm.message.bytecode_len;
                 assert data = evm.message.bytecode;
+                tempvar range_check_ptr = range_check_ptr;
             } else {
+                tempvar is_in_bounds = is_le(offset.low + size.low, evm.return_data_len);
+                if (is_in_bounds == FALSE) {
+                    let (revert_reason_len, revert_reason) = Errors.outOfBoundsRead();
+                    let evm = EVM.stop(
+                        evm, revert_reason_len, revert_reason, Errors.EXCEPTIONAL_HALT
+                    );
+                    return evm;
+                }
                 assert data_len = evm.return_data_len;
                 assert data = evm.return_data;
+                tempvar range_check_ptr = range_check_ptr;
             }
         }
+        let range_check_ptr = [ap - 1];
         slice(sliced_data, data_len, data, offset.low, size.low);
 
         Memory.store_n(size.low, sliced_data, dest_offset.low);