bug in reading IGES file

Hello,

Just to mention that there is a bug in Interface_ParamSet::Append when reading the General section of an IGES file.
This method has as input a String, and try to store it inside some local array of characters.

The problem is in the following lines of code :

// .. Gestion locale des caracteres ..
Standard_Integer i;
if (thelnval + lnval + 1 > thelnres) {
// Reservation de caracteres insuffisante : d abord augmenter
Standard_Integer newres = thelnres*2;
char* newval = new char[newres];

As you can guess (even if you don't understand french comments), if the size of the string to add (lnval) tends to be greater than the current allocated size (thelnres), then it tries to do some reallocation job by growing the reserved size by twice.
Well I must admit that this is an original implementation of reallocation

And what if the string to add still have a greater size after multiplying by 2... ?
Yes, it will probaly crash soon or later because there is writing in memory non-allocated

Hope OCC team will correct this.

Regards.

Jean Michel

Alexander Tsvyashchenko's picture

... just to mention that I've encountered that bug on real-world IGES file, traced it down to this code & fixed it, then was going to submit the patch to this forum, but decided to search for similar posts first - and found at least two threads that mention this problem and its solution (yours and this one: http://www.opencascade.org/org/forum/thread_11453/)

Considering your post is almost 2 years old and another thread is about 1 year old, but the issue is still not fixed in 6.2.0 - it doesn't look like anyone would be really interested in my patch, though ;-)

Andrey Betenev's picture

Dear Alexander,

The bug reported in this thread has been fixed in OCCT 6.2.1 (see Release Notes at http://www.opencascade.org/getocc/whatsnew - bug 17099 - sorry that it was not clearly reported).

You can submit your corrections here, e.g. in the form of diff output -- even if not immediately getting into mainstream OCCT, your fix will be available to the community.

Alexander Tsvyashchenko's picture

Ok, great!

Thank you for the information and sorry for confusion ;-)

As the fix is already in 6.2.1 and mentioned at least two times on the forum, there's probably not that much reason to post the diff, but still here it is just in case ...

==============================================================================

diff -urN src/Interface/Interface_ParamSet.cxx D:\_dev\libs\OpenCASCADE6.2.0_x64\ros\src/Interface/Interface_ParamSet.cxx
--- src/Interface/Interface_ParamSet.cxx 2001-09-21 10:55:51.000000000 +0300
+++ D:\_dev\libs\OpenCASCADE6.2.0_x64\ros\src/Interface/Interface_ParamSet.cxx 2008-07-16 17:36:50.195312700 +0300
@@ -38,7 +38,7 @@
Standard_Integer i;
if (thelnval + lnval + 1 > thelnres) {
// Reservation de caracteres insuffisante : d abord augmenter
- Standard_Integer newres = thelnres*2;
+ Standard_Integer newres = std::max(thelnres*2, thelnval + lnval + 1);
char* newval = new char[newres];
for (i = 0; i < thelnval; i ++) newval[i] = theval[i]; //szv#4:S4163:12Mar99 `<= thelnres` was wrong
// et cepatou : il faut realigner les Params deja enregistres sur

==============================================================================