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

@@ -223,6 +223,7 @@ enum class ErrorCode {
   kerCorruptedMetadata,
   kerArithmeticOverflow,
   kerMallocFailed,
+  kerInvalidIconvEncoding,
 
   kerErrorCount,
 };

src/convert.cpp

@@ -589,7 +589,13 @@ void Converter::cnvExifComment(const char* from, const char* to) {
     return;
   }
   // Todo: Convert to UTF-8 if necessary
+  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

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<size_t>(Exiv2::ErrorCode::kerErrorCount),
               "errList needs to contain a error msg for every ErrorCode defined in error.hpp");

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

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'
+""", 
+""""""]

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 = [