MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0022970Community[OCCT] OCCT:Visualizationpublic2012-02-02 03:412014-01-22 12:52
Reporterpdolbey 
Assigned Toaba 
PrioritynormalSeverityminor 
StatusclosedResolutionfixed 
PlatformAOSLOS VersionL
Product Version[OCCT] 6.5.2 
Target Version[OCCT] 6.6.0Fixed in Version[OCCT] 6.6.0 
Summary0022970: Incorrect array use in Graphic3d_StructureManager.cxx
DescriptionIn Graphic3d_StructureManager we see the code

/* Initialize PHIGS and start up */
        if (Initialisation) {

                Initialisation = Standard_False;
                /* table to manage IDs of StructureManager */
                for (i=1; i<=Limit; i++) StructureManager_ArrayId[i] = 0;

                StructureManager_CurrentId = 1;
                StructureManager_ArrayId[1] = 1;

        }
        else {
                for (i=1; i<=Limit && NotFound; i++)
                        if (StructureManager_ArrayId[i] == 0) {
                                NotFound = Standard_False;
                                StructureManager_CurrentId = i;
                                StructureManager_ArrayId[i] = 1;
                        }

                if (NotFound)
                        Graphic3d_InitialisationError::Raise
                                        ("Too many ViewManagers are defined");
        }

The for loops run from 1 to <=Limit (which is 100), but this is not FORTRAN :). In c++ arrays are accessed from 0 to <Limit - this error runs the risk of overwriting memory. How this incorrect use of array indexing propagates into the rest of the code needs to be checked.

Pete
Steps To ReproduceNone
TagsNo tags attached.
Test case numberNot needed
Attached Files

- Relationships
child of 0022832closedaba Community Not documented limitation of Graphic3d_StructureManager 

-  Notes
(0019339)
abv (manager)
2012-02-02 08:02

We actually have a potential out-of-bound problem here, since function Limit() returns StructureManager_MAX, which is the size of the StructureManager_ArrayId array. Hence the last step of the cycle will go behind the array limits.

Even if the out-of-bound problem can be fixed easily, having static array of fixed size is nonsense nowadays. Please consider if it is needed at all.
(0021933)
san (developer)
2012-10-24 20:46

It might be helpful if the exception message is changed to something clearer, e.g. "You are trying to create too many ViewManagers at the same time! The number of simultaneously created ViewManagers cannot exceed NNN" - where NNN should be replaced by Limit value.

If IDs are re-used by view managers if one is destroyed and another one is then created (this is to be checked), the current logic can be kept. Naturally, array indexing mistake (index starts from 0 and ends at Limit - 1) should be corrected.
(0021939)
aba (developer)
2012-10-25 12:29

It was checked and confirmed that after destruction of one view manager its ID will be re-used.
(0021943)
aba (developer)
2012-10-25 14:15

Bounds of loops in lines 96 and 103 were corrected according to bounds of the array StructureManager_ArrayId.
Initialisation error message was changed to more informative one.
(0021944)
aba (developer)
2012-10-25 14:16

The Git branch CR22970 is ready to be reviewed.
Please review.
(0021979)
san (developer)
2012-10-26 23:53

Branch CR22970 reviewed without remarks, ready for testing.
(0022079)
apn (administrator)
2012-11-02 15:00

Dear BugMaster,
Branch CR22970 (and products from GIT master) was compiled on Linux and Windows platforms and tested.

Regression:
Not detected

Improvements:
Not detected

Testing case:
Not needed

- Related Changesets
occt: master f163f612
Timestamp: 2012-10-25 10:07:11
Author: aba
Details ] Diff ]
0022970: Incorrect array use in Graphic3d_StructureManager.cxx

Corrected bounds of loops and initialisation error message.
mod - src/Graphic3d/Graphic3d_StructureManager.cxx Diff ] File ]

- Issue History
Date Modified Username Field Change
2012-02-02 03:41 pdolbey New Issue
2012-02-02 03:41 pdolbey Assigned To => san
2012-02-02 07:48 abv Relationship added child of 0022832
2012-02-02 08:02 abv Note Added: 0019339
2012-10-20 08:14 abv Target Version => 6.6.0
2012-10-24 11:19 san Status new => assigned
2012-10-24 11:19 san Assigned To san => aba
2012-10-24 20:46 san Note Added: 0021933
2012-10-25 12:29 aba Note Added: 0021939
2012-10-25 14:15 aba Note Added: 0021943
2012-10-25 14:16 aba Note Added: 0021944
2012-10-25 14:16 aba Status assigned => resolved
2012-10-25 14:16 aba Assigned To aba => san
2012-10-26 23:53 san Note Added: 0021979
2012-10-26 23:53 san Assigned To san => bugmaster
2012-10-26 23:53 san Status resolved => reviewed
2012-11-02 15:00 apn Note Added: 0022079
2012-11-02 15:01 apn Test case number => Not needed
2012-11-02 15:01 apn Status reviewed => tested
2012-11-16 13:03 aba Changeset attached => occt master f163f612
2012-11-16 13:03 aba Assigned To bugmaster => aba
2012-11-16 13:03 aba Status tested => verified
2012-11-16 13:03 aba Resolution open => fixed
2012-12-10 17:16 aba Changeset attached => occt master f163f612
2013-04-23 13:36 user533 Status verified => closed
2013-04-29 15:21 user533 Fixed in Version => 6.6.0
2014-01-22 12:52 abv Relationship added has duplicate 0024551


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker