Skip to content

Commit

Permalink
Fix a buffer overflow in restart summary read
Browse files Browse the repository at this point in the history
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.

Also fixed that it was always assumed that the restart filename
was relative.

Before, many restart cases would have been ignored because the file
was too long (or incorrectly assumed to be relative). Now, restart
data will most likely be included, which could lead to unexpected changed
behavior.
  • Loading branch information
eivindjahren committed Jan 25, 2024
1 parent a949810 commit eb005f9
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 32 deletions.
69 changes: 37 additions & 32 deletions lib/resdata/rd_smspec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -888,10 +889,20 @@ static void rd_smspec_load_restart(rd_smspec_type *rd_smspec,
}
#endif

std::string path = rd::util::path::dirname(rd_smspec->header_file);
smspec_header = rd_alloc_exfilename(path.c_str(), restart_base,
RD_SUMMARY_HEADER_FILE,
rd_smspec->formatted, 0);
std::string path = "";
if (util_is_abs_path(restart_base)) {
path = rd::util::path::dirname(restart_base);
std::string base = rd::util::path::basename(restart_base);
smspec_header = rd_alloc_exfilename(path.c_str(), base.c_str(),
RD_SUMMARY_HEADER_FILE,
rd_smspec->formatted, 0);
} else {
path = rd::util::path::dirname(rd_smspec->header_file);
smspec_header = rd_alloc_exfilename(path.c_str(), restart_base,
RD_SUMMARY_HEADER_FILE,
rd_smspec->formatted, 0);
}

if (smspec_header) {
if (!util_same_file(
smspec_header,
Expand Down Expand Up @@ -1198,14 +1209,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)) {

Expand All @@ -1226,20 +1230,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 "
Expand Down
2 changes: 2 additions & 0 deletions python/tests/rd_tests/test_sum.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down

0 comments on commit eb005f9

Please sign in to comment.