From 1f364be1fa9573848396e3c06019e5c019e9e9a1 Mon Sep 17 00:00:00 2001 From: Peter Date: Tue, 1 Nov 2022 13:33:36 +0000 Subject: [PATCH] Fix seg fault when using `iconv_open()` (#2403) * Fix seg fault when using `iconv_open()` - Fix failure condition for `iconv_open()` - Add new exception when failing to change the text encoding of an Exif comment * Add testing for `iconv_open()` seg fault bug * Fix Python test by changing log level --- include/exiv2/error.hpp | 1 + src/convert.cpp | 10 +++++-- src/error.cpp | 1 + src/value.cpp | 6 +++-- test/data/issue_2403_poc.exv | Bin 0 -> 79 bytes tests/bugfixes/github/test_issue_2403.py | 25 ++++++++++++++++++ .../test_regression_allfiles.py | 1 + 7 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 test/data/issue_2403_poc.exv create mode 100644 tests/bugfixes/github/test_issue_2403.py diff --git a/include/exiv2/error.hpp b/include/exiv2/error.hpp index 8ef0f841..ccd0e706 100644 --- a/include/exiv2/error.hpp +++ b/include/exiv2/error.hpp @@ -223,6 +223,7 @@ enum class ErrorCode { kerCorruptedMetadata, kerArithmeticOverflow, kerMallocFailed, + kerInvalidIconvEncoding, kerErrorCount, }; diff --git a/src/convert.cpp b/src/convert.cpp index 2db26fb2..0c441b7e 100644 --- a/src/convert.cpp +++ b/src/convert.cpp @@ -589,7 +589,13 @@ void Converter::cnvExifComment(const char* from, const char* to) { return; } // Todo: Convert to UTF-8 if necessary - (*xmpData_)[to] = cv->comment(); + try { + (*xmpData_)[to] = cv->comment(); + } catch (const Error&) { +#ifndef SUPPRESS_WARNINGS + EXV_WARNING << "Failed to convert " << from << " to " << to << "\n"; +#endif + } if (erase_) exifData_->erase(pos); } @@ -1567,7 +1573,7 @@ bool convertStringCharsetIconv(std::string& str, const char* from, const char* t bool ret = true; iconv_t cd; cd = iconv_open(to, from); - if (!cd) { + if (cd == (iconv_t)(-1)) { #ifndef SUPPRESS_WARNINGS EXV_WARNING << "iconv_open: " << strError() << "\n"; #endif diff --git a/src/error.cpp b/src/error.cpp index f2e212c5..c4ce1f2e 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -89,6 +89,7 @@ constexpr std::array errList{ N_("corrupted image metadata"), // kerCorruptedMetadata N_("Arithmetic operation overflow"), // kerArithmeticOverflow N_("Memory allocation failed"), // kerMallocFailed + N_("Cannot convert text encoding from '%1' to '%2'"), // kerInvalidIconvEncoding }; static_assert(errList.size() == static_cast(Exiv2::ErrorCode::kerErrorCount), "errList needs to contain a error msg for every ErrorCode defined in error.hpp"); diff --git a/src/value.cpp b/src/value.cpp index 5c08d9ff..3a9edabb 100644 --- a/src/value.cpp +++ b/src/value.cpp @@ -370,10 +370,11 @@ size_t CommentValue::copy(byte* buf, ByteOrder byteOrder) const { std::ostream& CommentValue::write(std::ostream& os) const { CharsetId csId = charsetId(); + std::string text = comment(); if (csId != undefined) { os << "charset=" << CharsetInfo::name(csId) << " "; } - return os << comment(); + return os << text; } std::string CommentValue::comment(const char* encoding) const { @@ -384,7 +385,8 @@ std::string CommentValue::comment(const char* encoding) const { c = value_.substr(8); if (charsetId() == unicode) { const char* from = !encoding || *encoding == '\0' ? detectCharset(c) : encoding; - convertStringCharset(c, from, "UTF-8"); + if (!convertStringCharset(c, from, "UTF-8")) + throw Error(ErrorCode::kerInvalidIconvEncoding, encoding, "UTF-8"); } bool bAscii = charsetId() == undefined || charsetId() == ascii; // # 1266 Remove trailing nulls diff --git a/test/data/issue_2403_poc.exv b/test/data/issue_2403_poc.exv new file mode 100644 index 0000000000000000000000000000000000000000..b1e8e2c1fd3405f928c4aa7ade4d22748fbf7cd5 GIT binary patch literal 79 zcmey*=vt9kX7v9dgA0(6#=zj|sl~v-z`(%BklD@xBpDc_fEWZA8QLbXGY9}#I$%Bn XL#Uspv%iZgLkL4ELoq`M!~dHAlNk># literal 0 HcmV?d00001 diff --git a/tests/bugfixes/github/test_issue_2403.py b/tests/bugfixes/github/test_issue_2403.py new file mode 100644 index 00000000..0144590c --- /dev/null +++ b/tests/bugfixes/github/test_issue_2403.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- + +from system_tests import CaseMeta, CopyTmpFiles, check_no_ASAN_UBSAN_errors +@CopyTmpFiles("$data_path/issue_2403_poc.exv") + +class checkIconvSegFault(metaclass=CaseMeta): + url = """""" + description = """Test the fixcom action in the exiv2 app""" + + filename = """$tmp_path/issue_2403_poc.exv""" + + commands = ["""$exiv2 --verbose --log e --encode made_up_encoding fixcom $filename""", + """$exiv2 --verbose --keep --encode UCS-2LE fixcom $filename"""] + retval = [1,0] + + stdout = ["""File 1/1: $filename +""", +"""File 1/1: $filename +Setting Exif UNICODE user comment to "Test" +"""] + + stderr = ["""Exiv2 exception in fixcom action for file $filename: +Cannot convert text encoding from 'made_up_encoding' to 'UTF-8' +""", +""""""] diff --git a/tests/regression_tests/test_regression_allfiles.py b/tests/regression_tests/test_regression_allfiles.py index 01304742..25ba4c0e 100644 --- a/tests/regression_tests/test_regression_allfiles.py +++ b/tests/regression_tests/test_regression_allfiles.py @@ -147,6 +147,7 @@ def get_valid_files(data_dir): # this test file actually contains some eixf info, but windows has # different output let's try and fix this later "exiv2-bug1044.tif", + "issue_2403_poc.exv", ] file_paths = [