View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0017100CommunityOCCT:Visualizationpublic2007-09-28 13:352012-03-29 17:26
ReporterabvAssigned Tobugmaster  
PrioritynormalSeveritytrivial 
Status closedResolutionfixed 
OSAll 
Target Version6.5.3Fixed in Version6.5.3 
Summary0017100: [OCC Forum] Buffer overflow vulnerability and loading TKOpenGl without environment variables on unix systems
DescriptionThis problem has been reported by Jan Brüninghaus on OCC Forum thread 12122
(http://www.opencascade.org/org/forum/thread_12122):

--------

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;

}


------------


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

[...]

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);
}
Additional information
and documentation updates		Documentation remark, added by APL 2011-04-27 15:08:24:

Changes:
The TKOpenGl graphics library can be defined by CSF_GraphicShr environment variable, otherwise it will be search with system's library path.

There are following changes in packages:
Draw package:
  1) Possible buffer overflow in Draw_Appli method has been corrected.
     Now the string, which defines the /DrawResources/DrawDefaults path
     could be more than 128 symbols length.

Graphic3d package:
  1) The ability to load TKOpenGl library without CSF_GraphicShr environment variables defined has been added to the Graphic3d_GraphicDevice. Now, if there is no CSF_GraphicShr variable found, the library is searched in system's libraries paths.
  2) private method Standard_Boolean Graphic3d_GraphicDevice::ShrIsDefined (TCollection_AsciiString& aShr) const declaration was modified to TCollection_AsciiString ShrEnvString() const to reflect current logics: This method now always returns graphics driver's string for loading it from environment path or system library path;
TagsNo tags attached.
Test case numberTest case is not required

Attached Files

  • apl-OCC17100-v1.tar.gz (6,651 bytes)
  • apl-OCC17100-v2.tar.gz (6,676 bytes)

Relationships

related to 0022952 closeddbv Memory leak in Draw.cxx (line 284) 

Activities

2011-04-27 17:06

 

apl-OCC17100-v1.tar.gz (6,651 bytes)

2011-04-28 15:54

 

apl-OCC17100-v2.tar.gz (6,676 bytes)

abv

2011-12-08 15:04

manager   ~0018784

Dear Anton,

Please check if the bug is still relevant, and either close it or provide a fix on current version in SVN

apl

2011-12-28 10:37

developer   ~0019027

The bug is still reproducible:
 - The buffer overflow exception raised in DRAW after following actions:
   1) unset DRAWDEFAULT variable;
   2) set CASROOT variable string length more than 128 characters;
   3) run DRAWEXE.

Additionally the fix allows to load libTKOpenGl.so library from LD_LIBRARY_PATH if CASROOT and CSF_GraphicShr are unset.
Without it, the TKOpenGl loading would be aborted with message "CASROOT and CSF_GraphicShr variables not set, aborting..."

apl

2011-12-28 10:40

developer   ~0019028

Dear Bugmaster,

The fix "apl-OCC17100-v2" has been merged in SVN branch
http://svn/svn/occt/branches/OCC17100

Branch is ready to be reviewed

apl

2011-12-28 10:45

developer   ~0019029

Dear Kirill,

Could you please review the modifications in SVN branch http://svn/svn/occt/branches/OCC17100

kgv

2011-12-28 17:12

developer   ~0019036

Last edited: 2011-12-28 17:13

 2apl,

could you please update this portion of code to use OSD_Environment and TCollection_AsciiString (including Graphic3d_GraphicDevice::ShrIsDefined (TCollection_AsciiString& aShr)) rather than ugly pointers manipulations?

apl

2012-01-30 09:29

developer   ~0019301

Code was updated with minimal modifications in logic:
- TCollection_AsciiString replaced the char pointers in Draw.cxx
- TCollection_AsciiString and OSD_Environment used in ShrIsDefined;
- private method Standard_Boolean Graphic3d_GraphicDevice::ShrIsDefined (TCollection_AsciiString& aShr) const declaration was modified to TCollection_AsciiString ShrEnvString() const to reflect current logics:
  This method now always returns graphics driver's string for loading it from environment path or system library path;

apl

2012-01-30 09:31

developer   ~0019302

Dear Kirill,

Please, could you review the new modifications?

SVN branch http://svn/svn/occt/branches/OCC17100

kgv

2012-01-31 10:18

developer   ~0019318

Bug branch passed review without remarks.
Dear Bugmaster, please perform regression tests.

apn

2012-02-02 13:30

administrator   ~0019352

Last edited: 2012-02-03 13:35

 Dear BugMaster,
Workbench KAS:dev:apn-17100-occt was created from SVN branch http://svn/svn/occt/branches/OCC17100
(and apn-17100-products from trunk) and compiled on Linux and Windows platforms.

There are not regressions in apn-17100-products regarding to KAS:dev:products-20120127-opt

There is unstable test:
che 003 C1
(It's unstable 'cause launching it in EMAX we get good results "This shape seems to be valid")

If CSF_GraphicShr is unset and CASROOT="/QAdisk/KAS/dev/apn-17100-occt-shortname" (more than 128 symbols or less, it's not important) and we trying to launch DRAWEXE(vinit), we get exception:

An exception was caught 0xb64ad650 : Aspect_GraphicDeviceDefinitionError: /dn47/KAS/dev/apn-17100-occt-shortname/Linux/lib/libTKOpenGl.so: cannot open shared object file: No such file or directory
** Exception ** 0xb64ad650 : Aspect_GraphicDeviceDefinitionError: /dn47/KAS/dev/apn-17100-occt-shortname/Linux/lib/libTKOpenGl.so: cannot open shared object file: No such file or directory

But LD_LIBRARY_PATH=...$CASROOT/lin/lib...
Trying to find "libTKOpenGl.so" on nonexisting way (paste "Linux").

Should be deleted module with CASROOT
 
See results in /QADisk/occttests/results/KAS/dev/ apn-17100-products_31012012/lin
See reference results in /QADisk/occttests/results/KAS/dev/products-20120127-opt_28012012/lin
See test cases in /QADisk/occttests/tests/ED

apl

2012-02-03 14:58

developer   ~0019380

Dear Sergey,

Could you please review the modifications?
The code of loading graphics driver from CASROOT environment variable is eliminated.

The SVN branch http://svn/svn/occt/branches/OCC17100 is ready to be reviewed

san

2012-02-06 20:19

developer   ~0019411

The branch reviewed without remarks, please test it.

apn

2012-02-08 10:08

administrator   ~0019448

Dear BugMaster,
 Workbench KAS:dev:apn-17100-occt was updated from SVN branch http://svn/svn/occt/branches/OCC17100
 (and apn-17100-products from trunk) and compiled on Linux and Windows platforms.
 
 There are not regressions in apn-17100-products regarding to KAS:dev:products-20120203-opt
 
After this update there are no exceptions (launching DRAWEXE (vinit)) when length of string, which defines the /DrawResources/DrawDefaults path, considers more than 128 symbols.

 See results in /QADisk/occttests/results/KAS/dev/ apn-17100-products_07022012/lin
 See reference results in /QADisk/occttests/results/KAS/dev/products-20120203-opt_03022012/lin
 See test cases in /QADisk/occttests/tests/ED

abv

2012-02-08 10:54

manager   ~0019451

Please consider possibility to create test case for this issue

bugmaster

2012-02-10 14:07

administrator   ~0019504

Integrated into trunk of occt repository

Date: 2012-02-10 13:51:01 +0400 (Fri, 10 Feb 2012)
New Revision: 10406

Modified:
   trunk/src/Draw/Draw.cxx
   trunk/src/Graphic3d/Graphic3d_GraphicDevice.cdl
   trunk/src/Graphic3d/Graphic3d_GraphicDevice.cxx

Related Changesets

occt: master 86be4295

2012-02-10 09:51:01

apl


Committer: bugmaster Details Diff		 0017100: [OCC Forum] Buffer overflow vulnerability and loading TKOpenGl without environment variables on unix systems Affected Issues
0017100
mod - src/Draw/Draw.cxx Diff File
mod - src/Graphic3d/Graphic3d_GraphicDevice.cdl Diff File
mod - src/Graphic3d/Graphic3d_GraphicDevice.cxx Diff File

Issue History

Date Modified Username Field Change
2010-10-06 19:03 bugmaster Status closed => assigned
2010-10-06 19:03 bugmaster Resolution suspended => @0@
2010-10-06 19:03 bugmaster Assigned To bugmaster => san
2011-04-07 18:11 bugmaster Assigned To san => apl
2011-04-27 17:08 apl Status assigned => resolved
2011-08-02 11:24 bugmaster Category OCCT:VIZ => OCCT:Visualization
2011-12-01 11:17 szy Fixed in Version EMPTY =>
2011-12-01 11:17 szy Description Updated
2011-12-01 11:17 szy Additional Information Updated
2011-12-08 15:04 abv Note Added: 0018784
2011-12-08 15:04 abv Status resolved => assigned
2011-12-28 10:37 apl Note Added: 0019027
2011-12-28 10:40 apl Note Added: 0019028
2011-12-28 10:40 apl Status assigned => resolved
2011-12-28 10:43 apl Assigned To apl => kgv
2011-12-28 10:45 apl Note Added: 0019029
2011-12-28 17:12 kgv Note Added: 0019036
2011-12-28 17:13 kgv Note Edited: 0019036
2011-12-28 17:13 kgv Assigned To kgv => apl
2011-12-28 17:13 kgv Status resolved => assigned
2012-01-30 09:29 apl Note Added: 0019301
2012-01-30 09:29 apl Status assigned => resolved
2012-01-30 09:31 apl Note Added: 0019302
2012-01-30 09:32 apl Assigned To apl => kgv
2012-01-30 09:36 apl Additional Information Updated
2012-01-31 10:18 kgv Note Added: 0019318
2012-01-31 10:18 kgv Assigned To kgv => bugmaster
2012-01-31 10:18 kgv Status resolved => reviewed
2012-02-02 13:30 apn Note Added: 0019352
2012-02-02 13:30 apn Test case number => Test case is not required
2012-02-02 13:34 apn Note Edited: 0019352
2012-02-03 13:35 apn Note Edited: 0019352
2012-02-03 13:36 apn Assigned To bugmaster => apl
2012-02-03 13:36 apn Status reviewed => assigned
2012-02-03 14:58 apl Note Added: 0019380
2012-02-03 14:58 apl Assigned To apl => san
2012-02-03 14:58 apl Status assigned => resolved
2012-02-03 15:03 apl Additional Information Updated
2012-02-03 15:07 apl Additional Information Updated
2012-02-06 20:19 san Note Added: 0019411
2012-02-06 20:19 san Assigned To san => bugmaster
2012-02-06 20:19 san Status resolved => reviewed
2012-02-08 10:08 apn Note Added: 0019448
2012-02-08 10:08 apn Status reviewed => tested
2012-02-08 10:54 abv Note Added: 0019451
2012-02-10 14:07 bugmaster Target Version => 6.5.3
2012-02-10 14:07 bugmaster Note Added: 0019504
2012-02-10 14:07 bugmaster Status tested => verified
2012-02-10 14:07 bugmaster Assigned To bugmaster => apl
2012-02-15 13:23 dbv Relationship added related to 0022952
2012-03-29 17:26 bugmaster Changeset attached => occt master 86be4295