Buffer overflow vulnerability and loading TKOpenGl without environment variables on unix systems

In the function Graphic3d_GraphicDevice::ShrIsDefined in the file src/Graphic3d/Graphic3d_GraphicDevice.cxx is a buffer overflow vulnerability, when TKOpenGl is loaded by using CASROOT and adding i.e. /lib/Linux/TKOpenGl.so. If the resulting path of the library exceeds 127 characters, the allocated memory is not enough.

I have corrected this behaviour as you can see below, by calculating the actual string length and then allocating the necessary memory. It also is now unnecessary to define CSF_GraphicShr or CASROOT for loading TKOpenGl as long as the library is in a path ldconfig is aware of. On Windows OCC behaves similar.

Standard_Boolean Graphic3d_GraphicDevice::ShrIsDefined (Standard_CString& aShr) const {

char *glso, *glul, *pkno;
char *glshr, *casroot;

casroot = getenv("CASROOT");
glso = getenv("CSF_GraphicShr");
glul = getenv("GRAPHICHOME");
pkno = getenv("CSF_Graphic3dLib");

#if defined(__hpux) || defined(HPUX)
const char *lib_path = "/lib/";
const char *lib_name = "libTKOpenGl.sl";
#elif defined(WNT)
const char *lib_path = "/";
const char *lib_name = "TKOpenGl.dll";
#else
const char *lib_path = "/lib/";
const char *lib_name = "libTKOpenGl.so";
#endif

if (! BAD(glso)) {
glshr = getenv("CSF_GraphicShr");
printf("Using graphic library defined through CSF_GraphicShr: %s\n", glshr);
} else if (! BAD(casroot)) {
// calculate the stringlen to avoid overflows
size_t str_length = strlen(casroot);

struct utsname info;
uname (&info);
str_length += strlen(info.sysname);

str_length += strlen(lib_path);
str_length += strlen(lib_name);
str_length += 2; // for \0 and "/"

glshr = (char *) malloc(str_length * sizeof(char));
if (!glshr) {
printf("Unalbe to allocate enough memory for graphic libraray name, defined through CASROOT\n");
return Standard_False;
}

glshr[0] = '\0';
strcat(glshr, casroot);
strcat(glshr,"/");
strcat(glshr,info.sysname);
strcat(glshr, lib_path);
strcat(glshr, lib_name);
printf("Using graphic library defined through CASROOT: %s\n", glshr);
} else {
glshr = lib_name;
//printf("You have not defined CSF_GraphicShr or CASROOT, aborting...");
//return Standard_False;
printf("Using system default TKOpenGl.so... %s\n", glshr);
}

aShr = glshr;

return Standard_True;

}

Jan Brüninghaus's picture

And the same in src/Draw/Draw.cxx....

Index: src/Draw/Draw.cxx
===================================================================
--- src/Draw/Draw.cxx (Revision 30)
+++ src/Draw/Draw.cxx (Arbeitskopie)
@@ -241,10 +241,14 @@
#endif
} else {

- char* thedefault = (char *) malloc (128);
+ const char *loc = "/src/DrawResources/DrawDefault";
+ size_t str_length = strlen(casroot);
+ str_length += strlen(loc);
+ str_length += 1; // for \0
+ char* thedefault = (char *) malloc(strlen * sizeof(char));
thedefault[0] = '\0';
strcat(thedefault,casroot);
- strcat (thedefault,"/src/DrawResources/DrawDefault");
+ strcat (thedefault, loc);
ReadInitFile(thedefault);
}

Jan Brüninghaus's picture

Hups, one type. Here is the correct one:

Index: src/Draw/Draw.cxx
===================================================================
--- src/Draw/Draw.cxx (Revision 30)
+++ src/Draw/Draw.cxx (Arbeitskopie)
@@ -241,10 +241,14 @@
#endif
} else {

- char* thedefault = (char *) malloc (128);
+ const char *loc = "/src/DrawResources/DrawDefault";
+ size_t str_length = strlen(casroot);
+ str_length += strlen(loc);
+ str_length += 1; // for \0
+ char* thedefault = (char *) malloc(str_length * sizeof(char));
thedefault[0] = '\0';
strcat(thedefault,casroot);
- strcat (thedefault,"/src/DrawResources/DrawDefault");
+ strcat (thedefault, loc);
ReadInitFile(thedefault);
}