View Issue Details

IDProjectCategoryView StatusDate SubmittedLast Update
0000669CommunityOCCT:Application Frameworkpublic2002-09-02 19:192011-12-15 17:54
ReporteremoAssigned Tosrn 
PrioritynormalSeveritytrivial 
Status closedResolutionfixed 
OSAll 
Fixed in Version5.0.0 
Summary0000669: Standard_GUID("HoleFeature") cause stack overwrite
DescriptionThis bug has been added by fchina at
http://www.opencascade.org/forumorg/bug.php?bug_id=75&f=8 .

I have found a suttle bug in OCC, and the misleading of SampleOcaf. Since
SampleOcaf extensively use Standard_GUID("BoxDriver"), this is a big misleading
for programmers who use OCAF. I have find the logic error why using
Standard_GUID("BoxDriver") will cause crash sometimes, and why it will generate
different string when stored in file with Debug version and Release version.

Look at this function in Standard_GUID.cxx.

Standard_Integer Standard_GUID_MatchChar(const Standard_CString buffer, const
Standard_Character aChar)
{
  Standard_CString tmpbuffer = buffer;
  Standard_Integer result = -1;

  while(*tmpbuffer != '\0' && *tmpbuffer != aChar) {tmpbuffer++; result++;}

  if (result >= 0) result++;

  return result;
}

Is there any trouble with this MatchChar ? it just look for aChar in buffer.
The problem is when aChar is not in buffer, the result will beyond the scope of
buffer!

A serious programming should check the bounding condition, it should check the
ending condition, i.e. if *tmpbuffer == '\0', set result = -1. As above code, if
aChar not in buffer, result will point to the char next to '\0', it's dangerous!

Look these two functions: (it is called in constructor)

Standard_CString Standard_GUID_GetValue32(const Standard_CString tmpBuffer,
Standard_Integer& my32b)
{
  Standard_Character strtmp[Standard_GUID_SIZE_ALLOC];
  Standard_Integer pos = 0;

  pos = Standard_GUID_MatchChar(tmpBuffer,'-');
  if (pos >= 0) {
    strncpy(strtmp,tmpBuffer,pos);
    strtmp[pos] = '\0';
    my32b = (Standard_Integer) strtoul(strtmp, (char **)NULL, 16);
  }

  return &tmpBuffer[pos+1];
}

Standard_CString Standard_GUID_GetValue16(const Standard_CString tmpBuffer,
Standard_Integer& my32b)
{
  Standard_Character strtmp[Standard_GUID_SIZE_ALLOC];
  Standard_Integer pos = 0;

  pos = Standard_GUID_MatchChar(tmpBuffer,'-');
  if (pos >= 0) {
    strncpy(strtmp,tmpBuffer,pos);
    strtmp[pos] = '\0';
    my32b = (Standard_Integer) strtoul(strtmp, (char **)NULL, 16);
  }

  return &tmpBuffer[pos+1];
}

No problem, isn't it?

Then assume when construct a Standard_GUID("HoleFeature") in stack. (this is a
common case just as SampleOcaf) by the constructor:

Standard_GUID::Standard_GUID(const Standard_CString aGuid)
: my32b ( 0),
  my16b1 ( 0),
  my16b2 ( 0),
  my16b3 ( 0),
  my8b1 ( 0),
  my8b2 ( 0),
  my8b3 ( 0),
  my8b4 ( 0),
  my8b5 ( 0),
  my8b6 ( 0)
{
  Standard_CString tmpBuffer = aGuid;
   
  tmpBuffer = Standard_GUID_GetValue32(tmpBuffer,my32b);
  tmpBuffer = Standard_GUID_GetValue16(tmpBuffer,my16b1);
  tmpBuffer = Standard_GUID_GetValue16(tmpBuffer,my16b2);
  tmpBuffer = Standard_GUID_GetValue16(tmpBuffer,my16b3);
  tmpBuffer = Standard_GUID_GetValue8(tmpBuffer,my8b1);
  tmpBuffer = Standard_GUID_GetValue8(tmpBuffer,my8b2);
  tmpBuffer = Standard_GUID_GetValue8(tmpBuffer,my8b3);
  tmpBuffer = Standard_GUID_GetValue8(tmpBuffer,my8b4);
  tmpBuffer = Standard_GUID_GetValue8(tmpBuffer,my8b5);
  tmpBuffer = Standard_GUID_GetValue8(tmpBuffer,my8b6);
}

The parameter passed into Standard_GUID is "HoleFeature", no '-', so tmpBuffer
has already pointed to incorrect content! And unfortunely, all those const
string in stack is allocated in some static segment by compiler, and this time
following "HoleFeature" is a long string, its size greater than
Standard_GUID_SIZE_ALLOC,
so when execute this code:

    strncpy(strtmp,tmpBuffer,pos);

    auto variable pos is overwrtten by strncpy functin! Debugger display pos as
a big integer( in hex, just a part of that long string and begin with length of
Standard_GUID_SIZE_ALLOC, so prove a buffer overwritting.), then

   strtmp[pos] = '\0';

   cause OS throw access violation.


The solution for OCC developer should of course check the loop end condition to
see *tmpBuffer = '\0'. if it is, return -1.
And in construtor should add a check: CheckGUIDFormat, if return false, throw
exception. (at least #define in debug version)

For application developer, change all code using Standard_GUID that follows
SampleOCAF, and always use tools to generate GUID and copy them into code. .

fhchina
TagsNo tags attached.
Test case number

Attached Files

Relationships

related to 0000738 closedptv Open CASCADE The GUIDs in XCAFDoc have incorrect format. 
related to 0000739 closedsrn Open CASCADE Invalid GUIDs in DDataStd_DrawPresentation and DDataStd_Sample. 

Activities

2002-09-05 07:39

 

10_SampleOcaf.zip (239,248 bytes)

2002-09-11 14:32

 

Standard_GUID.zip (2,073 bytes)

2002-09-17 15:22

 

OCC669-OCC738-OCC739.zip (9,471 bytes)

Issue History

Date Modified Username Field Change
2002-09-03 12:43 bugmaster Assigned To bugmaster => szy
2002-09-03 12:43 bugmaster Status new => assigned
2002-09-03 12:43 bugmaster Summary => Standard_GUID("HoleFeature") cause stack overwrite
2002-09-03 18:12 szy Assigned To szy => srn
2002-09-05 11:42 srn Status assigned => resolved
2002-09-11 18:45 bugmaster CC => apv
2002-09-11 19:47 apv CC => aki
2002-09-17 13:14 vtn OtherBugsDependingOnThis => 738, 739
2002-09-23 18:37 aki Status resolved => tested
2002-10-01 17:43 bugmaster Status tested => closed
2002-10-01 17:43 bugmaster Resolution @0@ => fixed
2011-08-02 10:32 bugmaster Category OCCT:OCAF => OCCT:Application Framework
2011-12-15 17:54 bugmaster Project Open CASCADE => Community