fly/flightgear
1
0
Fork 0
Code Activity

Fix various mp bugs.

Browse source 
In multiplaymgr.cxx:
The length of a string property that is too big was written out in the message
even though the string was truncated. Also, it was possible to overrun the
message buffer.

In incoming  messages, null-terminate the callsign to prevent any funny
business. Don't believe invalid string lengths. Turn the warning about unknown
properties from a warning into info; there are too many buggy clients out there,
not to mention people who add their own MP properties (hi vivian :)
This commit is contained in:
Tim Moore 2009-02-05 23:18:26 +01:00
parent bfc059aa03
commit e83caa6321
1 changed files with 77 additions and 84 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

161
src/MultiPlayer/multiplaymgr.cxx
View file

@ -302,19 +302,18 @@ namespace
// to change... // to change...
uint32_t length = XDR_decode_uint32(*xdr); uint32_t length = XDR_decode_uint32(*xdr);
xdr++; xdr++;
if ((length > 0) && (length < MAX_TEXT_SIZE)) { // Old versions truncated the string but left the length
xdr += length; // unadjusted.
// Now handle the padding if (length > MAX_TEXT_SIZE)
while ((length % 4) != 0) length = MAX_TEXT_SIZE;
{ xdr += length;
xdr++; // Now handle the padding
length++; while ((length % 4) != 0)
//cout << "0"; {
} xdr++;
} else { length++;
// The string appears to be invalid; bail. //cout << "0";
return false; }
}
} }
break; break;
default: default:
@ -480,37 +479,38 @@ FGMultiplayMgr::SendMyPosition(const FGExternalMotionData& motionInfo)
PosMsg.linearAccel[i] = XDR_encode_float (motionInfo.linearAccel(i)); PosMsg.linearAccel[i] = XDR_encode_float (motionInfo.linearAccel(i));
for (unsigned i = 0 ; i < 3; ++i) for (unsigned i = 0 ; i < 3; ++i)
PosMsg.angularAccel[i] = XDR_encode_float (motionInfo.angularAccel(i)); PosMsg.angularAccel[i] = XDR_encode_float (motionInfo.angularAccel(i));
// ensure alignment of message buffer
char Msg[MAX_PACKET_SIZE]; union {
memcpy(Msg + sizeof(T_MsgHdr), &PosMsg, sizeof(T_PositionMsg)); xdr_data2_t double_val;
char Msg[MAX_PACKET_SIZE];
} msgBuf;
memcpy(msgBuf.Msg + sizeof(T_MsgHdr), &PosMsg, sizeof(T_PositionMsg));
char* ptr = Msg + sizeof(T_MsgHdr) + sizeof(T_PositionMsg); xdr_data_t* ptr = reinterpret_cast<xdr_data_t*>(msgBuf.Msg + sizeof(T_MsgHdr)
+ sizeof(T_PositionMsg));
std::vector<FGPropertyData*>::const_iterator it; std::vector<FGPropertyData*>::const_iterator it;
it = motionInfo.properties.begin(); it = motionInfo.properties.begin();
//cout << "OUTPUT PROPERTIES\n"; //cout << "OUTPUT PROPERTIES\n";
while (it != motionInfo.properties.end() xdr_data_t* msgEnd = reinterpret_cast<xdr_data_t*>(msgBuf.Msg
&& ptr + 2 * sizeof(xdr_data_t) < (Msg + MAX_PACKET_SIZE)) { + MAX_PACKET_SIZE);
while (it != motionInfo.properties.end() && ptr + 2 < msgEnd) {
// First elements is the ID
xdr_data_t xdr = XDR_encode_uint32((*it)->id);
memcpy(ptr, &xdr, sizeof(xdr_data_t));
ptr += sizeof(xdr_data_t);
// First element is the ID. Write it out when we know we have room for
// the whole property.
xdr_data_t id = XDR_encode_uint32((*it)->id);
// The actual data representation depends on the type // The actual data representation depends on the type
switch ((*it)->type) { switch ((*it)->type) {
case SGPropertyNode::INT: case SGPropertyNode::INT:
case SGPropertyNode::BOOL: case SGPropertyNode::BOOL:
case SGPropertyNode::LONG: case SGPropertyNode::LONG:
xdr = XDR_encode_uint32((*it)->int_value); *ptr++ = id;
memcpy(ptr, &xdr, sizeof(xdr_data_t)); *ptr++ = XDR_encode_uint32((*it)->int_value);
ptr += sizeof(xdr_data_t);
//cout << "Prop:" << (*it)->id << " " << (*it)->type << " "<< (*it)->int_value << "\n"; //cout << "Prop:" << (*it)->id << " " << (*it)->type << " "<< (*it)->int_value << "\n";
break; break;
case SGPropertyNode::FLOAT: case SGPropertyNode::FLOAT:
case SGPropertyNode::DOUBLE: case SGPropertyNode::DOUBLE:
xdr = XDR_encode_float((*it)->float_value);; *ptr++ = id;
memcpy(ptr, &xdr, sizeof(xdr_data_t)); *ptr++ = XDR_encode_float((*it)->float_value);
ptr += sizeof(xdr_data_t);
//cout << "Prop:" << (*it)->id << " " << (*it)->type << " "<< (*it)->float_value << "\n"; //cout << "Prop:" << (*it)->id << " " << (*it)->type << " "<< (*it)->float_value << "\n";
break; break;
case SGPropertyNode::STRING: case SGPropertyNode::STRING:
@ -527,26 +527,24 @@ FGMultiplayMgr::SendMyPosition(const FGExternalMotionData& motionInfo)
// Add the length // Add the length
////cout << "String length: " << strlen(lcharptr) << "\n"; ////cout << "String length: " << strlen(lcharptr) << "\n";
uint32_t len = strlen(lcharptr); uint32_t len = strlen(lcharptr);
if (len > MAX_TEXT_SIZE)
len = MAX_TEXT_SIZE;
// XXX This should not be using 4 bytes per character! // XXX This should not be using 4 bytes per character!
if (ptr + (1 + len + (4 - len % 4)) * sizeof (xdr_data_t) // If there's not enough room for this property, drop it
>= (Msg + MAX_PACKET_SIZE)) // on the floor.
if (ptr + 2 + ((len + 3) & ~3) > msgEnd)
goto escape; goto escape;
//cout << "String length unint32: " << len << "\n"; //cout << "String length unint32: " << len << "\n";
xdr = XDR_encode_uint32(len); *ptr++ = id;
memcpy(ptr, &xdr, sizeof(xdr_data_t)); *ptr++ = XDR_encode_uint32(len);
ptr += sizeof(xdr_data_t);
if (len != 0) if (len != 0)
{ {
// Now the text itself // Now the text itself
// XXX This should not be using 4 bytes per character! // XXX This should not be using 4 bytes per character!
int lcount = 0; int lcount = 0;
while ((*lcharptr != '\0') && (lcount < MAX_TEXT_SIZE)) while ((*lcharptr != '\0') && (lcount < MAX_TEXT_SIZE))
{ {
xdr = XDR_encode_int8(*lcharptr); *ptr++ = XDR_encode_int8(*lcharptr);
memcpy(ptr, &xdr, sizeof(xdr_data_t));
ptr += sizeof(xdr_data_t);
lcharptr++; lcharptr++;
lcount++; lcount++;
} }
@ -556,9 +554,7 @@ FGMultiplayMgr::SendMyPosition(const FGExternalMotionData& motionInfo)
// Now pad if required // Now pad if required
while ((lcount % 4) != 0) while ((lcount % 4) != 0)
{ {
xdr = XDR_encode_int8(0); *ptr++ = XDR_encode_int8(0);
memcpy(ptr, &xdr, sizeof(xdr_data_t));
ptr += sizeof(xdr_data_t);
lcount++; lcount++;
//cout << "0"; //cout << "0";
} }
@ -569,20 +565,17 @@ FGMultiplayMgr::SendMyPosition(const FGExternalMotionData& motionInfo)
else else
{ {
// Nothing to encode // Nothing to encode
xdr = XDR_encode_uint32(0); *ptr++ = id;
memcpy(ptr, &xdr, sizeof(xdr_data_t)); *ptr++ = XDR_encode_uint32(0);
ptr += sizeof(xdr_data_t);
//cout << "Prop:" << (*it)->id << " " << (*it)->type << " 0\n"; //cout << "Prop:" << (*it)->id << " " << (*it)->type << " 0\n";
} }
} }
break; break;
default: default:
//cout << " Unknown Type: " << (*it)->type << "\n"; //cout << " Unknown Type: " << (*it)->type << "\n";
xdr = XDR_encode_float((*it)->float_value);; *ptr++ = id;
memcpy(ptr, &xdr, sizeof(xdr_data_t)); *ptr++ = XDR_encode_float((*it)->float_value);;
ptr += sizeof(xdr_data_t);
//cout << "Prop:" << (*it)->id << " " << (*it)->type << " "<< (*it)->float_value << "\n"; //cout << "Prop:" << (*it)->id << " " << (*it)->type << " "<< (*it)->float_value << "\n";
break; break;
} }
@ -592,10 +585,11 @@ FGMultiplayMgr::SendMyPosition(const FGExternalMotionData& motionInfo)
escape: escape:
T_MsgHdr MsgHdr; T_MsgHdr MsgHdr;
FillMsgHdr(&MsgHdr, POS_DATA_ID, ptr - Msg); unsigned msgLen = reinterpret_cast<char*>(ptr) - msgBuf.Msg;
memcpy(Msg, &MsgHdr, sizeof(T_MsgHdr)); FillMsgHdr(&MsgHdr, POS_DATA_ID, msgLen);
memcpy(msgBuf.Msg, &MsgHdr, sizeof(T_MsgHdr));
mSocket->sendto(Msg, ptr - Msg, 0, &mServer); mSocket->sendto(msgBuf.Msg, msgLen, 0, &mServer);
SG_LOG(SG_NETWORK, SG_DEBUG, "FGMultiplayMgr::SendMyPosition"); SG_LOG(SG_NETWORK, SG_DEBUG, "FGMultiplayMgr::SendMyPosition");
} // FGMultiplayMgr::SendMyPosition() } // FGMultiplayMgr::SendMyPosition()
////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////
@ -663,7 +657,11 @@ FGMultiplayMgr::Update(void)
////////////////////////////////////////////////// //////////////////////////////////////////////////
int bytes; int bytes;
do { do {
char Msg[MAX_PACKET_SIZE]; // ensure alignment
union {
xdr_data2_t doubleVal;
char Msg[MAX_PACKET_SIZE];
} msgBuf;
////////////////////////////////////////////////// //////////////////////////////////////////////////
// Although the recv call asks for // Although the recv call asks for
// MAX_PACKET_SIZE of data, the number of bytes // MAX_PACKET_SIZE of data, the number of bytes
@ -671,7 +669,8 @@ FGMultiplayMgr::Update(void)
// packet waiting to be processed. // packet waiting to be processed.
////////////////////////////////////////////////// //////////////////////////////////////////////////
netAddress SenderAddress; netAddress SenderAddress;
bytes = mSocket->recvfrom(Msg, sizeof(Msg), 0, &SenderAddress); bytes = mSocket->recvfrom(msgBuf.Msg, sizeof(msgBuf.Msg), 0,
&SenderAddress);
////////////////////////////////////////////////// //////////////////////////////////////////////////
// no Data received // no Data received
////////////////////////////////////////////////// //////////////////////////////////////////////////
@ -688,12 +687,13 @@ FGMultiplayMgr::Update(void)
////////////////////////////////////////////////// //////////////////////////////////////////////////
// Read header // Read header
////////////////////////////////////////////////// //////////////////////////////////////////////////
T_MsgHdr* MsgHdr = (T_MsgHdr *)Msg; T_MsgHdr* MsgHdr = reinterpret_cast<T_MsgHdr *>(msgBuf.Msg);
MsgHdr->Magic = XDR_decode_uint32 (MsgHdr->Magic); MsgHdr->Magic = XDR_decode_uint32 (MsgHdr->Magic);
MsgHdr->Version = XDR_decode_uint32 (MsgHdr->Version); MsgHdr->Version = XDR_decode_uint32 (MsgHdr->Version);
MsgHdr->MsgId = XDR_decode_uint32 (MsgHdr->MsgId); MsgHdr->MsgId = XDR_decode_uint32 (MsgHdr->MsgId);
MsgHdr->MsgLen = XDR_decode_uint32 (MsgHdr->MsgLen); MsgHdr->MsgLen = XDR_decode_uint32 (MsgHdr->MsgLen);
MsgHdr->ReplyPort = XDR_decode_uint32 (MsgHdr->ReplyPort); MsgHdr->ReplyPort = XDR_decode_uint32 (MsgHdr->ReplyPort);
MsgHdr->Callsign[MAX_CALLSIGN_LEN -1] = '\0';
if (MsgHdr->Magic != MSG_MAGIC) { if (MsgHdr->Magic != MSG_MAGIC) {
SG_LOG( SG_NETWORK, SG_ALERT, "FGMultiplayMgr::MP_ProcessData - " SG_LOG( SG_NETWORK, SG_ALERT, "FGMultiplayMgr::MP_ProcessData - "
<< "message has invalid magic number!" ); << "message has invalid magic number!" );
@ -705,8 +705,8 @@ FGMultiplayMgr::Update(void)
break; break;
} }
if (MsgHdr->MsgLen != bytes) { if (MsgHdr->MsgLen != bytes) {
SG_LOG( SG_NETWORK, SG_ALERT, "FGMultiplayMgr::MP_ProcessData - " SG_LOG(SG_NETWORK, SG_ALERT, "FGMultiplayMgr::MP_ProcessData - "
<< "message has invalid length!" ); << "message from " << MsgHdr->Callsign << " has invalid length!");
break; break;
} }
////////////////////////////////////////////////// //////////////////////////////////////////////////
@ -714,10 +714,10 @@ FGMultiplayMgr::Update(void)
////////////////////////////////////////////////// //////////////////////////////////////////////////
switch (MsgHdr->MsgId) { switch (MsgHdr->MsgId) {
case CHAT_MSG_ID: case CHAT_MSG_ID:
ProcessChatMsg(Msg, SenderAddress); ProcessChatMsg(msgBuf.Msg, SenderAddress);
break; break;
case POS_DATA_ID: case POS_DATA_ID:
ProcessPosMsg(Msg, SenderAddress, bytes, stamp); ProcessPosMsg(msgBuf.Msg, SenderAddress, bytes, stamp);
break; break;
case UNUSABLE_POS_DATA_ID: case UNUSABLE_POS_DATA_ID:
case OLD_OLD_POS_DATA_ID: case OLD_OLD_POS_DATA_ID:
@ -754,13 +754,14 @@ void
FGMultiplayMgr::ProcessPosMsg(const char *Msg, netAddress & SenderAddress, FGMultiplayMgr::ProcessPosMsg(const char *Msg, netAddress & SenderAddress,
unsigned len, long stamp) unsigned len, long stamp)
{ {
T_MsgHdr* MsgHdr = (T_MsgHdr *)Msg; const T_MsgHdr* MsgHdr = reinterpret_cast<const T_MsgHdr *>(Msg);
if (MsgHdr->MsgLen < sizeof(T_MsgHdr) + sizeof(T_PositionMsg)) { if (MsgHdr->MsgLen < sizeof(T_MsgHdr) + sizeof(T_PositionMsg)) {
SG_LOG( SG_NETWORK, SG_ALERT, "FGMultiplayMgr::MP_ProcessData - " SG_LOG( SG_NETWORK, SG_ALERT, "FGMultiplayMgr::MP_ProcessData - "
<< "Position message received with insufficient data" ); << "Position message received with insufficient data" );
return; return;
} }
T_PositionMsg* PosMsg = (T_PositionMsg *)(Msg + sizeof(T_MsgHdr)); const T_PositionMsg* PosMsg
= reinterpret_cast<const T_PositionMsg *>(Msg + sizeof(T_MsgHdr));
FGExternalMotionData motionInfo; FGExternalMotionData motionInfo;
motionInfo.time = XDR_decode_double(PosMsg->time); motionInfo.time = XDR_decode_double(PosMsg->time);
motionInfo.lag = XDR_decode_double(PosMsg->lag); motionInfo.lag = XDR_decode_double(PosMsg->lag);
@ -781,8 +782,7 @@ FGMultiplayMgr::ProcessPosMsg(const char *Msg, netAddress & SenderAddress,
//cout << "INPUT MESSAGE\n"; //cout << "INPUT MESSAGE\n";
xdr_data_t* xdr = (xdr_data_t*) const xdr_data_t* xdr = (xdr_data_t*)(Msg + sizeof(T_MsgHdr) + sizeof(T_PositionMsg));
(Msg + sizeof(T_MsgHdr) + sizeof(T_PositionMsg));
// There was a bug in 1.9.0 and before: T_PositionMsg was 196 bytes // There was a bug in 1.9.0 and before: T_PositionMsg was 196 bytes
// on 32 bit architectures and 200 bytes on 64 bit, and this // on 32 bit architectures and 200 bytes on 64 bit, and this
// structure is put directly on the wire. By looking at the padding, // structure is put directly on the wire. By looking at the padding,
@ -846,35 +846,27 @@ FGMultiplayMgr::ProcessPosMsg(const char *Msg, netAddress & SenderAddress,
uint32_t length = XDR_decode_uint32(*xdr); uint32_t length = XDR_decode_uint32(*xdr);
xdr++; xdr++;
//cout << length << " "; //cout << length << " ";
// Old versions truncated the string but left the length unadjusted.
if ((length > 0) && (length < MAX_TEXT_SIZE)) if (length > MAX_TEXT_SIZE)
{ length = MAX_TEXT_SIZE;
pData->string_value = new char[length + 1]; pData->string_value = new char[length + 1];
//cout << " String: "; //cout << " String: ";
for (int i = 0; i < length; i++)
for (int i = 0; i < length; i++)
{ {
pData->string_value[i] = (char) XDR_decode_int8(*xdr); pData->string_value[i] = (char) XDR_decode_int8(*xdr);
xdr++; xdr++;
//cout << pData->string_value[i]; //cout << pData->string_value[i];
} }
pData->string_value[length] = '\0'; pData->string_value[length] = '\0';
// Now handle the padding // Now handle the padding
while ((length % 4) != 0) while ((length % 4) != 0)
{ {
xdr++; xdr++;
length++; length++;
//cout << "0"; //cout << "0";
} }
}
else
{
pData->string_value = new char[1];
pData->string_value[0] = '\0';
}
//cout << "\n"; //cout << "\n";
} }
break; break;
@ -891,8 +883,9 @@ FGMultiplayMgr::ProcessPosMsg(const char *Msg, netAddress & SenderAddress,
else else
{ {
// We failed to find the property. We'll try the next packet immediately. // We failed to find the property. We'll try the next packet immediately.
SG_LOG(SG_NETWORK, SG_WARN, "FGMultiplayMgr::ProcessPosMsg - " SG_LOG(SG_NETWORK, SG_INFO, "FGMultiplayMgr::ProcessPosMsg - "
<< "found unknown property id" << pData->id); "message from " << MsgHdr->Callsign << " has unknown property id "
<< pData->id);
} }
} }
noprops: noprops: