diff --git a/ApplicationLibCode/UnitTests/RifReaderEclipseSummary-Test.cpp b/ApplicationLibCode/UnitTests/RifReaderEclipseSummary-Test.cpp index 4be9b6914f..ba53518385 100644 --- a/ApplicationLibCode/UnitTests/RifReaderEclipseSummary-Test.cpp +++ b/ApplicationLibCode/UnitTests/RifReaderEclipseSummary-Test.cpp @@ -18,6 +18,7 @@ #include "gtest/gtest.h" +#include "RiaStringEncodingTools.h" #include "RiaTestDataDirectory.h" #include "RifEclEclipseSummary.h" @@ -25,12 +26,19 @@ #include "RifReaderEclipseSummary.h" #include "RifSummaryReaderAggregator.h" +#include +#include +#include +#include + #include #include #include #include #include +#include +#include //-------------------------------------------------------------------------------------------------- /// @@ -78,6 +86,107 @@ TEST( RifEclipseSummaryToolsTest, FindSummaryFilesWithDirectoryNamedAsCase ) EXPECT_TRUE( RifEclipseSummaryTools::hasSummaryDataFiles( dir.filePath( "CASE.SMSPEC" ) ) ); } +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +static void writeIntKeyword( fortio_type* fortio, const char* header, const std::vector& values ) +{ + ecl_kw_type* kw = ecl_kw_alloc( header, (int)values.size(), ECL_INT ); + for ( int i = 0; i < (int)values.size(); i++ ) + ecl_kw_iset_int( kw, i, values[i] ); + ecl_kw_fwrite( kw, fortio ); + ecl_kw_free( kw ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +static void writeFloatKeyword( fortio_type* fortio, const char* header, const std::vector& values ) +{ + ecl_kw_type* kw = ecl_kw_alloc( header, (int)values.size(), ECL_FLOAT ); + for ( int i = 0; i < (int)values.size(); i++ ) + ecl_kw_iset_float( kw, i, values[i] ); + ecl_kw_fwrite( kw, fortio ); + ecl_kw_free( kw ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +static void writeCharKeyword( fortio_type* fortio, const char* header, const std::vector& values ) +{ + ecl_kw_type* kw = ecl_kw_alloc( header, (int)values.size(), ECL_CHAR ); + for ( int i = 0; i < (int)values.size(); i++ ) + ecl_kw_iset_string8( kw, i, values[i].c_str() ); + ecl_kw_fwrite( kw, fortio ); + ecl_kw_free( kw ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +TEST( RifEclipseSummaryToolsTest, OpenCorruptSmspecFile ) +{ + // Reproduces https://github.com/OPM/ResInsight/issues/14381: when a SMSPEC file is corrupt or + // partially written, a keyword can be present in the file index while reading its data fails. + // The lazy keyword loader then returns NULL, and ecl_smspec_fread_header dereferenced the NULL + // keyword. The import must fail gracefully instead. + QTemporaryDir tempDir; + ASSERT_TRUE( tempDir.isValid() ); + QDir dir( tempDir.path() ); + + QString smspecFileName = dir.filePath( "CASE.SMSPEC" ); + { + fortio_type* fortio = fortio_open_writer( RiaStringEncodingTools::toNativeEncoded( smspecFileName ).data(), false, ECL_ENDIAN_FLIP ); + ASSERT_TRUE( fortio != nullptr ); + + writeIntKeyword( fortio, DIMENS_KW, { 2, 1, 1, 1, 0, 0 } ); + writeCharKeyword( fortio, KEYWORDS_KW, { "TIME", "FOPT" } ); + writeCharKeyword( fortio, WGNAMES_KW, { ":+:+:+:+", ":+:+:+:+" } ); + writeCharKeyword( fortio, UNITS_KW, { "DAYS", "SM3" } ); + writeIntKeyword( fortio, STARTDAT_KW, { 1, 1, 2020 } ); + + // INTEHEAD is written last, and the data record of this keyword is corrupted below + writeIntKeyword( fortio, INTEHEAD_KW, { 1, 100 } ); + + fortio_fclose( fortio ); + } + + QString unsmryFileName = dir.filePath( "CASE.UNSMRY" ); + { + fortio_type* fortio = fortio_open_writer( RiaStringEncodingTools::toNativeEncoded( unsmryFileName ).data(), false, ECL_ENDIAN_FLIP ); + ASSERT_TRUE( fortio != nullptr ); + + writeIntKeyword( fortio, SEQHDR_KW, { 0 } ); + writeIntKeyword( fortio, MINISTEP_KW, { 0 } ); + writeFloatKeyword( fortio, PARAMS_KW, { 0.0f, 0.0f } ); + + fortio_fclose( fortio ); + } + + // Sanity check: the intact file pair must open successfully + { + ecl_sum_type* eclSum = RifEclipseSummaryTools::openEclSum( smspecFileName, false ); + ASSERT_TRUE( eclSum != nullptr ); + RifEclipseSummaryTools::closeEclSum( eclSum ); + } + + // Corrupt the length marker of the INTEHEAD data record, located 16 bytes before the end of the + // file (4 byte marker + 8 bytes data + 4 byte trailing marker). The file scan only reads keyword + // headers and skips over data, so the keyword is still present in the file index, while reading + // the keyword data fails and returns NULL. + { + QFile file( smspecFileName ); + ASSERT_TRUE( file.open( QIODevice::ReadWrite ) ); + ASSERT_TRUE( file.seek( file.size() - 16 ) ); + const char corruptMarker[4] = { 0x7f, 0x7f, 0x7f, 0x7f }; + ASSERT_EQ( 4, file.write( corruptMarker, 4 ) ); + } + + ecl_sum_type* eclSum = RifEclipseSummaryTools::openEclSum( smspecFileName, false ); + EXPECT_TRUE( eclSum == nullptr ); +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- diff --git a/ThirdParty/Ert/lib/ecl/ecl_smspec.cpp b/ThirdParty/Ert/lib/ecl/ecl_smspec.cpp index 4543d46c06..eab6a88579 100644 --- a/ThirdParty/Ert/lib/ecl/ecl_smspec.cpp +++ b/ThirdParty/Ert/lib/ecl/ecl_smspec.cpp @@ -1106,17 +1106,34 @@ static bool ecl_smspec_fread_header(ecl_smspec_type * ecl_smspec, const char * h ecl_kw_type *numly = NULL; ecl_kw_type *numlz = NULL; + /* + The keywords are present in the file index, but loading the keyword data + can fail if the file is truncated or corrupt; ecl_file_iget_named_kw() + then returns NULL. Fail gracefully instead of dereferencing NULL. + */ + if (!wells || !keywords || !startdat || !units || !dimens) { + ecl_file_close( header ); + return false; + } + int params_index; ecl_smspec->num_regions = 0; ecl_smspec->params_size = ecl_kw_get_size(keywords); - if (startdat == NULL) - util_abort("%s: could not locate STARTDAT keyword in header - aborting \n",__func__); - if (ecl_file_has_kw(header , NUMS_KW)) + if (ecl_file_has_kw(header , NUMS_KW)) { nums = ecl_file_iget_named_kw(header , NUMS_KW , 0); + if (!nums) { + ecl_file_close( header ); + return false; + } + } if (ecl_file_has_kw(header, INTEHEAD_KW)) { const ecl_kw_type * intehead = ecl_file_iget_named_kw(header, INTEHEAD_KW, 0); + if (!intehead) { + ecl_file_close( header ); + return false; + } ecl_smspec->unit_system = (ert_ecl_unit_enum)ecl_kw_iget_int(intehead, INTEHEAD_SMSPEC_UNIT_INDEX); /* The second item in the INTEHEAD vector is an integer designating which @@ -1130,6 +1147,10 @@ static bool ecl_smspec_fread_header(ecl_smspec_type * ecl_smspec, const char * h numlx = ecl_file_iget_named_kw( header , NUMLX_KW , 0 ); numly = ecl_file_iget_named_kw( header , NUMLY_KW , 0 ); numlz = ecl_file_iget_named_kw( header , NUMLZ_KW , 0 ); + if (!lgrs || !numlx || !numly || !numlz) { + ecl_file_close( header ); + return false; + } ecl_smspec->has_lgr = true; } else ecl_smspec->has_lgr = false; @@ -1206,8 +1227,11 @@ static bool ecl_smspec_fread_header(ecl_smspec_type * ecl_smspec, const char * h ecl_file_close( header ); return true; - } else + } else { + if (header) + ecl_file_close( header ); return false; + } }