The OFF import routine for ctmconv can access out of bounds

[musicinmybrain]

The easiest way to observe this is to compile with libstdc++ assertions enabled. In Makefile.linux, add -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS to the CPPFLAGS. (This is part of the distribution default compiler flags in Fedora).

Now,

$ make -f Makefile.linux
$ curl -L -O https://github.com/mikedh/trimesh/raw/4.2.4/models/headless.ctm
$ LD_LIBRARY_PATH="$PWD/lib" ./tools/ctmconv headless.ctm headless.off
Loading headless.ctm... 3.775 ms
Saving headless.off... 87.118 ms
$ LD_LIBRARY_PATH="$PWD/lib" ./tools/ctmconv headless.off foo.ctm
Loading headless.off... /usr/include/c++/13/bits/basic_string.h:1238: std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::const_reference std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator[](size_type) const [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>; const_reference = const char&; size_type = long unsigned int]: Assertion '__pos <= size()' failed.
Aborted (core dumped)

If we remove -s so the executable is not stripped of debug symbols:

OpenCTM/tools/Makefile.linux

Line 56 in 91b3b71

$(CPP) -s -o $@ -L$(OPENCTMDIR) -L$(TINYXMLDIR) $(CTMCONVOBJS) -Wl,-rpath,. -lopenctm -ltinyxml

…and furthermore add -g -Og to the CPPFLAGS, then recompile, then we can try again with gdb and get a really nice backtrace:

$ LD_LIBRARY_PATH="$PWD/lib" gdb --args ./tools/ctmconv headless.off foo.ctm
[…]
(gdb) bt
#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
#1  0x00007ffff7aae8a3 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
#2  0x00007ffff7a5c8ee in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007ffff7a448ff in __GI_abort () at abort.c:79
#4  0x00007ffff7cd95b0 in std::__glibcxx_assert_fail (file=file@entry=0x4258c0 "/usr/include/c++/13/bits/basic_string.h", line=line@entry=1238,
    function=function@entry=0x4258e8 "std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::const_reference std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator[](size_type) const [with _CharT = char; _Traits = std::char_traits<ch"..., condition=condition@entry=0x425798 "__pos <= size()") at ../../../../../libstdc++-v3/src/c++11/assert_fail.cc:41
#5  0x0000000000406ad0 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::operator[] (this=this@entry=0x7fffffffcc30, __pos=__pos@entry=18446744073709551615)
    at /usr/include/c++/13/bits/basic_string.h:1238
#6  0x000000000040689b in TrimString (aString="") at common.cpp:48
#7  0x000000000041be16 in ReadNextLine (aStream=..., aResult="OFF", aComment="Binary STL output from Blender: /home/marcus/Projekt/OpenCTM/tools/headlessgiant") at off.cpp:72
#8  0x000000000041c023 in Import_OFF (aFileName=aFileName@entry=0x7fffffffd370 "headless.off", aMesh=aMesh@entry=0x7fffffffd260) at off.cpp:114
#9  0x000000000040907f in ImportMesh (aFileName=0x7fffffffd370 "headless.off", aMesh=aMesh@entry=0x7fffffffd260) at meshio.cpp:68
#10 0x0000000000405b87 in main (argc=3, argv=0x7fffffffd548) at /usr/include/c++/13/bits/basic_string.h:222
(gdb) frame 6
#6  0x000000000040689b in TrimString (aString="") at common.cpp:48
48        while((p2 > p1) && IsWhiteSpace(aString[p2]))
(gdb) info locals
l = <optimized out>
p1 = 0
p2 = 18446744073709551615
(gdb) info args
aString = ""
(gdb) frame 7
#7  0x000000000041be16 in ReadNextLine (aStream=..., aResult="OFF", aComment="Binary STL output from Blender: /home/marcus/Projekt/OpenCTM/tools/headlessgiant") at off.cpp:72
72          aResult = TrimString(line);
(gdb) info locals
line = ""
commentPos = 0

We can see that when TrimString is passed an empty string, p2 wraps around to the largest size_t value…

OpenCTM/tools/common.cpp

Lines 42 to 45 in 91b3b71

	string TrimString(const string &aString)
	{
	size_t l = aString.size();
	size_t p1 = 0, p2 = l - 1;

…and the following loop accesses many bytes from the string…

OpenCTM/tools/common.cpp

Lines 46 to 47 in 91b3b71

	while((p1 < p2) && IsWhiteSpace(aString[p1]))
	++ p1;

…which is of course very bad, because the string is empty.