Cannot find WHY I am getting a Buffer Overrun.

Hi,

I have a problem with a Buffer overrun crashing my application. I have looked through the code to try and find what may be causing it and cannot work it out!! I am checking all the sizes for memory copies and string lengths before copying (at least as far as I know).

These are the "Offending" bits of code - I can of course supply more if required.

void get_trkQD_data(C_AfdxComplete *MsgTx_trkQD)
{
AiInt32 x = 0;
AiInt32 y = 0;
AiInt32 SoftmapId = 0;
AiInt32 CmdWrd = 0;
AiInt32 pad = 0;
char strCallsignA[200];

int testDecode = MsgTx_trkQD->DecodeData();

if(testDecode != FDX_ERR)
{
MsgTx_trkQD->DetermineFS();

// Position (lat/long)
MsgTx_trkQD->ReadData(1, "tSMIdWrdDSE", &SoftmapId, sizeof(AiInt32));
MsgTx_trkQD->ReadData(1, "tCmdWrdDSE", &CmdWrd, sizeof(AiInt32));
MsgTx_trkQD->ReadData(1, "tCallSignDSE", strCallsignA, sizeof(AiUInt8)*200);
MsgTx_trkQD->ReadData(1, "tXDisplayCoOrdDSE", &x, sizeof(AiInt32));
MsgTx_trkQD->ReadData(1, "tYDisplayCoOrdDSE", &y, sizeof(AiInt32));

printf("\nTrack Query Display data (Queueing):");
printf("\nSize =%d", MsgTx_trkQD->GetMsgSize());
printf("\nSoftmap : %d: Cmd Word %d: Callsign %s\n X: %d\t Y: %d",
SoftmapId, CmdWrd, strCallsignA, x, y);
}
}

The buffer overrun occurs at the end of the function, when return is attempted to the calling code. The overrun does not occur if the ReadData for the char array does not occur, or if the char array is renamed to str_Callsign, or if it is made bigger (202 characters for example).

The ReadData call ends up looking at the following code for the char array:

int C_AfdxMsgDSE::Read( void *paiData,
AiUInt32 aiBufferSize,
bool swap)
{
// Do a quick validity check on ptr
if(paiData == NULL) {
SetErrorStr("C_AfdxMsgDSE Read: Invalid buffer (Null)");
return FDX_ERR;
}

// Check DSE created correctly
if((m_pMsgBuffer == NULL) || (m_aiSize <= 0)) {
SetErrorStr("C_AfdxMsgDSE Read: Data Set Element incorrectly created");
return FDX_ERR;
}

// Check buffersize is at least the maximum for variable
// If fixed length then receieve buffer must be bigger/same size as expected
// For Strings this needs to incorporate the 2 byte for the length - hence is covered there
if((m_eiType != AFDX_STRING) && (aiBufferSize < m_aiSize)) {
SetErrorStr("C_AfdxMsgDSE Read: Insufficient buffer size");
return FDX_ERR;
}

// Set memory
memset(paiData, 0, m_aiSize);

// Check variable types
switch(m_eiType) {
case AFDX_STRING:
{
// Check buffersize is at least the maximum for variable
// If fixed length then receieve buffer must be bigger/same size as expected
// String is 2 bytes less than expected due to the length aspect
if(aiBufferSize < (m_aiSize-sizeof(AiInt16))) {
SetErrorStr("C_AfdxMsgDSE Read: Insufficient buffer size");
return FDX_ERR;
}

// Set memory
char *pStrLocn = (char*)m_pMsgBuffer+sizeof(AiInt16);
memcpy(&m_afdxStrLen, m_pMsgBuffer, sizeof(AiInt16));

// Must swap before using string length!
if(swap)
m_afdxStrLen = byte_swap_shortint(m_afdxStrLen);

// If zero then just set memory to zero's
if(m_afdxStrLen == 0) {
memset(paiData, 0, (m_aiSize-sizeof(AiInt16)));
// Copy just the string
} else if(m_afdxStrLen < aiBufferSize) {
strncpy((char*)paiData, pStrLocn, m_afdxStrLen);
// Copy it all
} else {
memcpy(paiData, pStrLocn, (m_aiSize-sizeof(AiInt16)));
}
}
break;
default:
// Set memory
memcpy(paiData, m_pMsgBuffer, m_aiSize);
if(swap)
SetErrorStr("C_AfdxMsgDSE Write: Cannot byteswap this Data Set Element");
break;
}

return FDX_OK;
}

This only really makes sense if you know that the definition for a ADFX_STRING is an 2byte unsigned short int to hold the string length, followed by the character string of variable length to the defined maximum.
This is defined when the string is set up:

int C_AfdxMsgDSE::Define( AiUInt32 aiDataSize_in,
Type_AfdxDataPrimType eiDataType_in,
const char *pName_DSE)
{
// Do a quick validity check on datasize
if(aiDataSize_in <= 0) {
SetErrorStr("C_AfdxMsgDSE Define: Data Size is invalid");
return FDX_ERR;
}

// Do a quick validity check on name
if(pName_DSE == NULL) {
if(m_strName.GetLength() <= 0) {
SetErrorStr("C_AfdxMsgDSE Define: Name is invalid");
return FDX_ERR;
}
pName_DSE = m_strName;
} else {
if(strlen(pName_DSE) <= 0) {
SetErrorStr("C_AfdxMsgDSE Define: Name is invalid");
return FDX_ERR;
}
}

// Ensure the datasize is acceptable
switch(eiDataType_in) {
case AFDX_STRING:
{
// Max size cannot be zero
if(aiDataSize_in <= 0) {
SetErrorStr("C_AfdxMsgDSE Define: Max size cannot be zero");
return FDX_ERR;
}

// Max length must be 2 byte aligned
if(aiDataSize_in % 2) {
SetErrorStr("C_AfdxMsgDSE Define: Max length must be 2 byte aligned");
return FDX_ERR;
}

// Increase size by 2 bytes for the length field
aiDataSize_in += sizeof(AiUInt16);

// Set the string length to default to 0
m_afdxStrLen = 0;
}
break;
case AFDX_NO_TYPE:
{
SetErrorStr("C_AfdxMsgDSE Define: Cannot define a no type DSE");
return FDX_ERR;
}
break;
default:
{
SetErrorStr("C_AfdxMsgDSE Define: No such DSE type");
return FDX_ERR;
}
break;
}

// Assign values and memory space
ReleaseMem();
m_strName = pName_DSE;
m_aiSize = (AiUInt32) aiDataSize_in;
m_eiType = eiDataType_in;
m_pMsgBuffer = (AiUInt8 *)malloc(aiDataSize_in);
memset(m_pMsgBuffer, 0, aiDataSize_in);

return FDX_OK;
}

Any help would be greatly appreciated, as I am now at my wits end and have very little hair left.
Thanks,
Confused
[6544 byte] By [ConfusedByWindows] at [2007-11-20 1:34:41]
# 1 Re: Cannot find WHY I am getting a Buffer Overrun.
Hi all,

I have fixed it. It is truely amazing what a bit of cardboard engineering can do for you!! Just after I posted it and was rereading it when I saw my error - I had even commented it!!

// Check buffersize is at least the maximum for variable
// If fixed length then receieve buffer must be bigger/same size as expected
// For Strings this needs to incorporate the 2 byte for the length - hence is covered there
if((m_eiType != AFDX_STRING) && (aiBufferSize < m_aiSize)) {
SetErrorStr("C_AfdxMsgDSE Read: Insufficient buffer size");
return FDX_ERR;
}

// Set memory
memset(paiData, 0, m_aiSize);

The correct code should be:

// Check buffersize is at least the maximum for variable
// If fixed length then receieve buffer must be bigger/same size as expected
// For Strings this needs to incorporate the 2 byte for the length
if(m_eiType != AFDX_STRING) {
if(aiBufferSize < m_aiSize) {
SetErrorStr("C_AfdxMsgDSE Read: Insufficient buffer size");
return FDX_ERR;
}
// Set memory
memset(paiData, 0, m_aiSize);
} else {
// String is 2 bytes less than expected due to the length aspect
if(aiBufferSize < (m_aiSize-sizeof(AiInt16))) {
SetErrorStr("C_AfdxMsgDSE Read: Insufficient buffer size");
return FDX_ERR;
}
// Set memory
memset(paiData, 0, (m_aiSize-sizeof(AiInt16)));
}

// Check variable types
switch(m_eiType) {
case AFDX_STRING:
{
// Set memory
char *pStrLocn = (char*)m_pMsgBuffer+sizeof(AiInt16);
memcpy(&m_afdxStrLen, m_pMsgBuffer, sizeof(AiInt16));

// Must swap before using string length!
if(swap)
m_afdxStrLen = byte_swap_shortint(m_afdxStrLen);

// If zero then just set memory to zero's
if(m_afdxStrLen == 0) {
memset(paiData, 0, (m_aiSize-sizeof(AiInt16)));
// Copy just the string
} else if(m_afdxStrLen < aiBufferSize) {
strncpy((char*)paiData, pStrLocn, m_afdxStrLen);
// Copy it all
} else {
memcpy(paiData, pStrLocn, (m_aiSize-sizeof(AiInt16)));
}
}
break;

Now I am not overrunning the input buffer (which is expecting just the string) with the length data as well! A definate case of not seeing the wood for the trees and probably being a bit too close to the code (as it were).

Anyway,
thanks to any who looked-in.

Confused
ConfusedByWindows at 2007-11-9 1:07:58 >