For all issues regarding the Forums use, please, refer to the Forum Rules.

Our Solutions

Need professional assistance?
Consider our:

Support Offerings

 

Need to speed up your development?
Have a look at our:

Samples & Tools

 

Need some functionality extending standard OCCT capabilities?
Check out our:

Adv. Components

Related pages

BUG: Standard_GUID("BoxDriver") may cause buffer overwrite.

fhchina's picture
Forums: 

I have found a suttle bug in OCC,
and the mislead of SampleOcaf. Thanks for the discussion with Stephane.

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!

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 proof 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. I have a lot of work to do. wu wu wu... :-(.

Again, thank Stephane for his inspiration.

fhchina