Standard_GUID("HoleFeature") cause stack overwrite

This 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