Skip to content

Commit

Permalink
Targa: String overflow safety (#3622)
Browse files Browse the repository at this point in the history
Added utility functions safe_string_view and safe_string to strutil.h.
  • Loading branch information
lgritz authored Oct 21, 2022
1 parent 5fa8d99 commit 19121dc
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 4 deletions.
16 changes: 16 additions & 0 deletions src/include/OpenImageIO/strutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions src/libutil/strutil_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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();
Expand Down
8 changes: 4 additions & 4 deletions src/targa.imageio/targainput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions testsuite/targa/ref/out.err.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions testsuite/targa/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Binary file added testsuite/targa/src/crash6.tga
Binary file not shown.

0 comments on commit 19121dc

Please sign in to comment.