From a918ef4ab76b8170b3e4204d80092a48c0c1f32b Mon Sep 17 00:00:00 2001 From: Eivind Jahren Date: Fri, 26 Jan 2024 07:11:25 +0100 Subject: [PATCH 1/2] Fix a buffer overflow in restart summary read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The number of blocks of 8 character strings in the RESTART keyword can now be more than 9, and in any case the file could always have the length set to something larger, leading to stack corruption. Co-authored-by: Håvard Berland --- lib/resdata/rd_smspec.cpp | 51 ++++++++++++++----------------- python/tests/rd_tests/test_sum.py | 2 ++ 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/lib/resdata/rd_smspec.cpp b/lib/resdata/rd_smspec.cpp index 8575b5df3..8e4ae4b9e 100644 --- a/lib/resdata/rd_smspec.cpp +++ b/lib/resdata/rd_smspec.cpp @@ -856,15 +856,16 @@ static void rd_smspec_load_restart(rd_smspec_type *rd_smspec, if (rd_file_has_kw(header, RESTART_KW)) { const rd_kw_type *restart_kw = rd_file_iget_named_kw(header, RESTART_KW, 0); - char tmp_base - [73]; /* To accomodate a maximum of 9 items which consist of 8 characters each. */ - char *restart_base; - int i; - tmp_base[0] = '\0'; - for (i = 0; i < rd_kw_get_size(restart_kw); i++) - strcat(tmp_base, (const char *)rd_kw_iget_ptr(restart_kw, i)); - - restart_base = util_alloc_strip_copy(tmp_base); + int num_blocks = rd_kw_get_size(restart_kw); + num_blocks = 0 ? num_blocks < 0 : num_blocks; + char *tmp_base = (char *)calloc(8 * num_blocks + 1, sizeof(char)); + for (int i = 0; i < num_blocks; i++) { + const char *part = (const char *)rd_kw_iget_ptr(restart_kw, i); + strncat(tmp_base, part, 8); + } + + char *restart_base = util_alloc_strip_copy(tmp_base); + free(tmp_base); if (strlen(restart_base)) { /* We ignore the empty ones. */ char *smspec_header; @@ -1198,14 +1199,7 @@ static bool rd_smspec_fread_header(rd_smspec_type *rd_smspec, rd_smspec_type *rd_smspec_fread_alloc(const char *header_file, const char *key_join_string, bool include_restart) { - rd_smspec_type *rd_smspec; - - { - char *path; - util_alloc_file_components(header_file, &path, NULL, NULL); - rd_smspec = rd_smspec_alloc_empty(false, key_join_string); - free(path); - } + rd_smspec_type *rd_smspec = rd_smspec_alloc_empty(false, key_join_string); if (rd_smspec_fread_header(rd_smspec, header_file, include_restart)) { @@ -1226,20 +1220,21 @@ rd_smspec_type *rd_smspec_fread_alloc(const char *header_file, const rd::smspec_node *day_node = rd_smspec_get_var_node(rd_smspec->misc_var_index, "DAY"); - if (day_node != NULL) { - rd_smspec->day_index = smspec_node_get_params_index(day_node); - rd_smspec->month_index = smspec_node_get_params_index( - &rd_smspec->misc_var_index["MONTH"]); - rd_smspec->year_index = smspec_node_get_params_index( - &rd_smspec->misc_var_index["YEAR"]); + const rd::smspec_node *month_node = + rd_smspec_get_var_node(rd_smspec->misc_var_index, "MONTH"); + const rd::smspec_node *year_node = + rd_smspec_get_var_node(rd_smspec->misc_var_index, "YEAR"); + + if (day_node != NULL && month_node != NULL && year_node != NULL) { + rd_smspec->day_index = day_node->get_params_index(); + rd_smspec->month_index = month_node->get_params_index(); + rd_smspec->year_index = year_node->get_params_index(); } if ((rd_smspec->time_index == -1) && (rd_smspec->day_index == -1)) { - /* Unusable configuration. - - Seems the restart file can also have time specified with - 'YEARS' as basic time unit; that mode is not supported. - */ + // Unusable configuration. + // Seems the restart file can also have time specified with + // 'YEARS' as basic time unit; that mode is not supported. util_abort("%s: Sorry the SMSPEC file seems to lack all time " "information, need either TIME, or DAY/MONTH/YEAR " diff --git a/python/tests/rd_tests/test_sum.py b/python/tests/rd_tests/test_sum.py index ab6ac8d44..cc777eb03 100644 --- a/python/tests/rd_tests/test_sum.py +++ b/python/tests/rd_tests/test_sum.py @@ -142,6 +142,8 @@ def test_identify_var_type(self): ) self.assertEqual(Summary.varType("RPR"), SummaryVarType.RD_SMSPEC_REGION_VAR) self.assertEqual(Summary.varType("WNEWTON"), SummaryVarType.RD_SMSPEC_MISC_VAR) + self.assertEqual(Summary.varType("YEAR"), SummaryVarType.RD_SMSPEC_MISC_VAR) + self.assertEqual(Summary.varType("MONTH"), SummaryVarType.RD_SMSPEC_MISC_VAR) self.assertEqual( Summary.varType("AARQ:4"), SummaryVarType.RD_SMSPEC_AQUIFER_VAR ) From 611d2588f669276a98d5e07bef48901874409926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5vard=20Berland?= Date: Fri, 26 Jan 2024 09:25:06 +0100 Subject: [PATCH 2/2] Format python code with black 24.1 --- python/resdata/__init__.py | 1 + python/resdata/geometry/__init__.py | 1 + python/resdata/geometry/cpolyline.py | 1 + python/resdata/geometry/cpolyline_collection.py | 1 + python/resdata/gravimetry/rd_subsidence.py | 1 + python/resdata/grid/rd_grid.py | 1 + python/resdata/grid/rd_region.py | 1 + python/resdata/rd_util.py | 1 + python/resdata/resfile/fortio.py | 1 + python/resdata/resfile/rd_file.py | 1 + python/resdata/summary/__init__.py | 1 - python/resdata/util/util/stringlist.py | 1 + python/tests/rd_tests/test_sum_equinor.py | 4 ++-- 13 files changed, 13 insertions(+), 3 deletions(-) diff --git a/python/resdata/__init__.py b/python/resdata/__init__.py index a78fe8d2c..fc1395a60 100644 --- a/python/resdata/__init__.py +++ b/python/resdata/__init__.py @@ -29,6 +29,7 @@ alternative fails, the loader will try the default load behaviour before giving up completely. """ + import ctypes as ct import os.path import sys diff --git a/python/resdata/geometry/__init__.py b/python/resdata/geometry/__init__.py index 83851e7d6..50e558a21 100644 --- a/python/resdata/geometry/__init__.py +++ b/python/resdata/geometry/__init__.py @@ -2,6 +2,7 @@ Simple package for working with 2D geometry. """ + import resdata from cwrap import Prototype diff --git a/python/resdata/geometry/cpolyline.py b/python/resdata/geometry/cpolyline.py index 31cd9ac73..559fe797a 100644 --- a/python/resdata/geometry/cpolyline.py +++ b/python/resdata/geometry/cpolyline.py @@ -1,6 +1,7 @@ """ Create a polygon """ + import ctypes import os.path diff --git a/python/resdata/geometry/cpolyline_collection.py b/python/resdata/geometry/cpolyline_collection.py index b5957466e..80608ac31 100644 --- a/python/resdata/geometry/cpolyline_collection.py +++ b/python/resdata/geometry/cpolyline_collection.py @@ -1,6 +1,7 @@ """ Create a polygon """ + import ctypes from cwrap import BaseCClass diff --git a/python/resdata/gravimetry/rd_subsidence.py b/python/resdata/gravimetry/rd_subsidence.py index 217443d87..0ad6b7bb8 100644 --- a/python/resdata/gravimetry/rd_subsidence.py +++ b/python/resdata/gravimetry/rd_subsidence.py @@ -6,6 +6,7 @@ different surveys. The implementation is a thin wrapper around the rd_subsidence.c implementation in the resdata library. """ + from cwrap import BaseCClass from resdata import ResdataPrototype from resdata.util.util import monkey_the_camel diff --git a/python/resdata/grid/rd_grid.py b/python/resdata/grid/rd_grid.py index 752d2a5fc..b1472c55c 100644 --- a/python/resdata/grid/rd_grid.py +++ b/python/resdata/grid/rd_grid.py @@ -7,6 +7,7 @@ implemented in the Grid class. The rd_grid module is a thin wrapper around the rd_grid.c implementation from the resdata library. """ + import ctypes import warnings diff --git a/python/resdata/grid/rd_region.py b/python/resdata/grid/rd_region.py index 8360fb0fe..79222208f 100644 --- a/python/resdata/grid/rd_region.py +++ b/python/resdata/grid/rd_region.py @@ -10,6 +10,7 @@ When the selection process is complete the region instance can be queried for the corresponding list of indices. """ + from functools import wraps import ctypes diff --git a/python/resdata/rd_util.py b/python/resdata/rd_util.py index 0060be3eb..50932e08f 100644 --- a/python/resdata/rd_util.py +++ b/python/resdata/rd_util.py @@ -9,6 +9,7 @@ In addition to the enum definitions there are a few stateless functions from rd_util.c which are not bound to any class type. """ + from __future__ import absolute_import import ctypes diff --git a/python/resdata/resfile/fortio.py b/python/resdata/resfile/fortio.py index bf77162b3..98dc7681b 100644 --- a/python/resdata/resfile/fortio.py +++ b/python/resdata/resfile/fortio.py @@ -21,6 +21,7 @@ support passing of FortIO handles to the underlying C functions. A more extensive wrapping of the fortio implementation would be easy. """ + import ctypes import os diff --git a/python/resdata/resfile/rd_file.py b/python/resdata/resfile/rd_file.py index ec107d7f6..04c074748 100644 --- a/python/resdata/resfile/rd_file.py +++ b/python/resdata/resfile/rd_file.py @@ -20,6 +20,7 @@ [1]: In particular for restart files, which do not have a special RestartFile class, there is some specialized functionality. """ + import ctypes import datetime import re diff --git a/python/resdata/summary/__init__.py b/python/resdata/summary/__init__.py index 4805d1d32..44cc63424 100644 --- a/python/resdata/summary/__init__.py +++ b/python/resdata/summary/__init__.py @@ -4,7 +4,6 @@ used as basis for queries on summary vectors. """ - import resdata.util.util import resdata.geometry diff --git a/python/resdata/util/util/stringlist.py b/python/resdata/util/util/stringlist.py index eb4a0911f..8cd6b2f55 100644 --- a/python/resdata/util/util/stringlist.py +++ b/python/resdata/util/util/stringlist.py @@ -15,6 +15,7 @@ return a normal python list of string objects, used in this way you hardly need to notice that the StringList class is at play. """ + from resdata import ResdataPrototype from cwrap import BaseCClass diff --git a/python/tests/rd_tests/test_sum_equinor.py b/python/tests/rd_tests/test_sum_equinor.py index 04d815679..0f46400dd 100755 --- a/python/tests/rd_tests/test_sum_equinor.py +++ b/python/tests/rd_tests/test_sum_equinor.py @@ -488,8 +488,8 @@ def test_ix_case(self): ) self.assertIsNotNone(eclipse_summary) - hwell_padder = ( - lambda key: key if key.split(":")[-1] != "HWELL_PR" else key + "OD" + hwell_padder = lambda key: ( + key if key.split(":")[-1] != "HWELL_PR" else key + "OD" ) self.assertEqual( intersect_summary.keys("WWCT*"),