Targa: String overflow safety

src/include/OpenImageIO/strutil.h

@@ -899,6 +899,22 @@ OIIO_UTIL_API char* safe_strcat(char *dst, string_view src, size_t size) noexcep
 OIIO_UTIL_API size_t safe_strlen(const char* str, size_t size) noexcept;
 
 
+/// Return a string_view that is a substring of the given C string, ending at
+/// the first null character or after size characters, whichever comes first.
+inline string_view safe_string_view(const char* str, size_t size) noexcept
+{
+    return string_view(str, safe_strlen(str, size));
+}
+
+/// Return a std::string that is a substring of the given C string, ending at
+/// the first null character or after size characters, whichever comes first.
+inline std::string safe_string(const char* str, size_t size)
+{
+    return safe_string_view(str, size);
+}
+
+
+
 /// Is the character a whitespace character (space, linefeed, tab, carrage
 /// return)? Note: this is safer than C isspace(), which has undefined
 /// behavior for negative char values. Also note that it differs from C

src/libutil/strutil_test.cpp

@@ -1112,6 +1112,36 @@ test_safe_strcat()
 
 
 
+// test safe_strlen and closely related safe_string_view, safe_string
+void
+test_safe_strlen()
+{
+    char a[] = "012";  // expected
+    char b[] = "012" "\0" "456789";  // nul embedded in the string
+    char c[] = "0123456789001234567890";  // long string
+    char d[] = "";  // empty string
+
+    Strutil::print("Testing safe_strlen\n");
+    OIIO_CHECK_EQUAL(Strutil::safe_strlen(a, 10), 3);
+    OIIO_CHECK_EQUAL(Strutil::safe_strlen(b, 10), 3);
+    OIIO_CHECK_EQUAL(Strutil::safe_strlen(c, 10), 10);
+    OIIO_CHECK_EQUAL(Strutil::safe_strlen(d, 10), 0);
+
+    std::cout << "Testing safe_string_view\n";
+    OIIO_CHECK_EQUAL(Strutil::safe_string_view(a, 10), string_view("012"));
+    OIIO_CHECK_EQUAL(Strutil::safe_string_view(b, 10), string_view("012"));
+    OIIO_CHECK_EQUAL(Strutil::safe_string_view(c, 10), string_view("0123456789"));
+    OIIO_CHECK_EQUAL(Strutil::safe_string_view(d, 10), string_view(""));
+
+    std::cout << "Testing safe_string\n";
+    OIIO_CHECK_EQUAL(Strutil::safe_string(a, 10), std::string("012"));
+    OIIO_CHECK_EQUAL(Strutil::safe_string(b, 10), std::string("012"));
+    OIIO_CHECK_EQUAL(Strutil::safe_string(c, 10), std::string("0123456789"));
+    OIIO_CHECK_EQUAL(Strutil::safe_string(d, 10), std::string(""));
+}
+
+
+
 // test some of the trickier methods in string_view.
 void
 test_string_view()
@@ -1565,6 +1595,7 @@ main(int /*argc*/, char* /*argv*/[])
     test_extract();
     test_safe_strcpy();
     test_safe_strcat();
+    test_safe_strlen();
     test_string_view();
     test_parse();
     test_locale();

src/targa.imageio/targainput.cpp

@@ -334,18 +334,18 @@ TGAInput::read_tga2_header()
             return false;
 
         // concatenate the lines into a single string
-        std::string tmpstr((const char*)buf.c);
+        std::string tmpstr = Strutil::safe_string((const char*)buf.c, 81);
         if (buf.c[81]) {
             tmpstr += "\n";
-            tmpstr += (const char*)&buf.c[81];
+            tmpstr += Strutil::safe_string((const char*)&buf.c[81], 81);
         }
         if (buf.c[162]) {
             tmpstr += "\n";
-            tmpstr += (const char*)&buf.c[162];
+            tmpstr += Strutil::safe_string((const char*)&buf.c[162], 81);
         }
         if (buf.c[243]) {
             tmpstr += "\n";
-            tmpstr += (const char*)&buf.c[243];
+            tmpstr += Strutil::safe_string((const char*)&buf.c[243], 81);
         }
         if (tmpstr.length() > 0)
             m_spec.attribute("ImageDescription", tmpstr);

testsuite/targa/ref/out.err.txt

@@ -6,3 +6,6 @@ Full command line was:
 oiiotool ERROR: read : "src/crash3.tga": Palette image with no palette
 Full command line was:
 > oiiotool -colorconfig ../common/OpenColorIO/nuke-default/config.ocio --oiioattrib try_all_readers 0 src/crash3.tga -o crash3.exr
+oiiotool ERROR: read : "src/crash6.tga": Read error: hit end of file
+Full command line was:
+> oiiotool -colorconfig ../common/OpenColorIO/nuke-default/config.ocio --oiioattrib try_all_readers 0 src/crash6.tga -o crash6.exr

testsuite/targa/run.py

@@ -17,6 +17,7 @@
 command += oiiotool("--oiioattrib try_all_readers 0 src/crash3.tga -o crash3.exr", failureok = True)
 command += oiiotool("--oiioattrib try_all_readers 0 src/crash4.tga -o crash4.exr", failureok = True)
 command += oiiotool("--oiioattrib try_all_readers 0 src/crash5.tga -o crash5.exr", failureok = True)
+command += oiiotool("--oiioattrib try_all_readers 0 src/crash6.tga -o crash6.exr", failureok = True)
 
 # Test odds and ends, unusual files
 command += rw_command("src", "1x1.tga")