Add configurable recv limits to protect against malicious peer

RecvBufferSize
RecvBufferMessages
RecvMaxMessageSize

P4:7421829,7421830

include/steam/steamnetworkingtypes.h

@@ -1135,6 +1135,28 @@ enum ESteamNetworkingConfigValue
 	/// Default is 512k (524288 bytes)
 	k_ESteamNetworkingConfig_SendBufferSize = 9,
 
+	/// [connection int32] Upper limit on total size (in bytes) of received messages
+	/// that will be buffered waiting to be processed by the application.  If this limit
+	/// is exceeded, packets will be dropped.  This is to protect us from a malicious
+	/// peer flooding us with messages faster than we can process them.
+	/// 
+	/// This must be bigger than k_ESteamNetworkingConfig_RecvMaxMessageSize
+	k_ESteamNetworkingConfig_RecvBufferSize = 47,
+
+	/// [connection int32] Upper limit on the number of received messages that will
+	/// that will be buffered waiting to be processed by the application.  If this limit
+	/// is exceeded, packets will be dropped.  This is to protect us from a malicious
+	/// peer flooding us with messages faster than we can pull them off the wire.
+	k_ESteamNetworkingConfig_RecvBufferMessages = 48,
+
+	/// [connection int32] Maximum message size that we are willing to receive.
+	/// if a client attempts to send us a message larger than this, the connection
+	/// will be immediately closed.
+	///
+	/// Default is 512k (524288 bytes).  Note that the peer needs to be able to
+	/// send a message this big.  (See k_cbMaxSteamNetworkingSocketsMessageSizeSend.)
+	k_ESteamNetworkingConfig_RecvMaxMessageSize = 49,
+
 	/// [connection int64] Get/set userdata as a configuration option.
 	/// The default value is -1.   You may want to set the user data as
 	/// a config value, instead of using ISteamNetworkingSockets::SetConnectionUserData

src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp

@@ -68,7 +68,10 @@ DEFINE_GLOBAL_CONFIGVAL( void *, Callback_FakeIPResult, nullptr );
 
 DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, TimeoutInitial, 10000, 0, INT32_MAX );
 DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, TimeoutConnected, 10000, 0, INT32_MAX );
-DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, SendBufferSize, 512*1024, 0, 0x10000000 );
+DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, SendBufferSize, 512*1024, 4*1024, 0x10000000 );
+DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, RecvBufferSize, 1024*1024, 4*1024, 0x10000000 );
+DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, RecvBufferMessages, 1000, 2, 0x10000000 );
+DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, RecvMaxMessageSize, 512*1024, 64, 0x10000000 );
 DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int64, ConnectionUserData, -1 ); // no limits here
 DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, SendRateMin, 128*1024, 1024, 0x10000000 );
 DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, SendRateMax, 1024*1024, 1024, 0x10000000 );

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp

@@ -170,7 +170,6 @@ void SteamNetworkingMessageQueue::AssertLockHeld() const
 
 void SteamNetworkingMessageQueue::PurgeMessages()
 {
-
 	AssertLockHeld();
 	while ( !empty() )
 	{
@@ -179,6 +178,7 @@ void SteamNetworkingMessageQueue::PurgeMessages()
 		Assert( m_pFirst != pMsg );
 		pMsg->Release();
 	}
+	Assert( m_nMessageCount == 0 );
 }
 
 int SteamNetworkingMessageQueue::RemoveMessages( SteamNetworkingMessage_t **ppOutMessages, int nMaxMessages )
@@ -2104,6 +2104,7 @@ int CSteamNetworkConnectionBase::APIReceiveMessages( SteamNetworkingMessage_t **
 	m_pLock->AssertHeldByCurrentThread();
 
 	g_lockAllRecvMessageQueues.lock();
+	
 	int result = m_queueRecvMessages.RemoveMessages( ppOutMessages, nMaxMessages );
 	g_lockAllRecvMessageQueues.unlock();
 
@@ -2606,7 +2607,27 @@ void CSteamNetworkConnectionBase::ConnectionStateChanged( ESteamNetworkingConnec
 
 CSteamNetworkingMessage *CSteamNetworkConnectionBase::AllocateNewRecvMessage( uint32 cbSize, int nFlags, SteamNetworkingMicroseconds usecNow )
 {
+	//
+	// Check limits
+	//
+
+	// Max message size
+	if ( (uint32)m_connectionConfig.m_RecvMaxMessageSize.Get() < cbSize )
+	{
+		SpewMsg( "[%s] recv message of size %u too large for limit of %d.\n", GetDescription(), cbSize, m_connectionConfig.m_RecvMaxMessageSize.Get() );
+		ConnectionState_ProblemDetectedLocally( k_ESteamNetConnectionEnd_Misc_InternalError, "Failed to allocate a buffer of size %u (limit is %d).", cbSize, m_connectionConfig.m_RecvMaxMessageSize.Get() );
+		return nullptr;
+	}
+
+	// Max number of messages buffered
+	if ( m_queueRecvMessages.m_nMessageCount >= m_connectionConfig.m_RecvBufferMessages.Get() )
+	{
+		SpewWarningRateLimited( usecNow, "[%s] recv queue overflow %d messages already queued.\n", GetDescription(), m_queueRecvMessages.m_nMessageCount );
+		return nullptr;
+	}
+
 	CSteamNetworkingMessage *pMsg = CSteamNetworkingMessage::New( cbSize );
+
 	if ( !pMsg )
 	{
 		// Failed!  if it's for a reliable message, then we must abort the connection.
@@ -2629,8 +2650,7 @@ bool CSteamNetworkConnectionBase::ReceivedMessageData( const void *pData, int cb
 	if ( !pMsg )
 	{
 		// Hm.  this failure really is probably a sign that we are in a pretty bad state,
-		// and we are unlikely to recover.  Should we just abort the connection?
-		// Right now, we'll try to muddle on.
+		// and we are unlikely to recover. Let the caller know about failure (this drops the connection).
 		return false;
 	}
 
@@ -2642,12 +2662,10 @@ bool CSteamNetworkConnectionBase::ReceivedMessageData( const void *pData, int cb
 	memcpy( pMsg->m_pData, pData, cbData );
 
 	// Receive it
-	ReceivedMessage( pMsg );
-
-	return true;
+	return ReceivedMessage( pMsg );
 }
 
-void CSteamNetworkConnectionBase::ReceivedMessage( CSteamNetworkingMessage *pMsg )
+bool CSteamNetworkConnectionBase::ReceivedMessage( CSteamNetworkingMessage *pMsg )
 {
 	m_pLock->AssertHeldByCurrentThread();
 
@@ -2661,7 +2679,7 @@ void CSteamNetworkConnectionBase::ReceivedMessage( CSteamNetworkingMessage *pMsg
 		if ( m_pMessagesEndPointSessionOwner )
 		{
 			m_pMessagesEndPointSessionOwner->ReceivedMessage( pMsg, this );
-			return;
+			return true;
 		}
 	#endif
 
@@ -2674,14 +2692,30 @@ void CSteamNetworkConnectionBase::ReceivedMessage( CSteamNetworkingMessage *pMsg
 	// which keeps this really simple.
 	g_lockAllRecvMessageQueues.lock();
 
+	Assert( pMsg->m_cbSize >= 0 );
+	if ( m_queueRecvMessages.m_nMessageCount >= m_connectionConfig.m_RecvBufferMessages.Get() )
+	{
+		g_lockAllRecvMessageQueues.unlock();
+		SpewWarningRateLimited ( SteamNetworkingSockets_GetLocalTimestamp(), "[%s] recv queue overflow %d messages already queued.\n", GetDescription(), m_queueRecvMessages.m_nMessageCount );
+		pMsg->Release();
+		return false;
+	}
+
+	if ( m_queueRecvMessages.m_nMessageSize + pMsg->m_cbSize > m_connectionConfig.m_RecvBufferSize.Get() )
+	{
+		g_lockAllRecvMessageQueues.unlock();
+		SpewWarningRateLimited ( SteamNetworkingSockets_GetLocalTimestamp(), "[%s] recv queue overflow %d + %d bytes exceeds limit of %d.\n", GetDescription(), m_queueRecvMessages.m_nMessageSize, pMsg->m_cbSize, m_connectionConfig.m_RecvBufferSize.Get() );
+		pMsg->Release();
+		return false;
+	}
+
 	// Add to end of my queue.
 	pMsg->LinkToQueueTail( &CSteamNetworkingMessage::m_links, &m_queueRecvMessages );
-
 	// Add to the poll group, if we are in one
 	if ( m_pPollGroup )
 		pMsg->LinkToQueueTail( &CSteamNetworkingMessage::m_linksSecondaryQueue, &m_pPollGroup->m_queueRecvMessages );
-
 	g_lockAllRecvMessageQueues.unlock();
+	return true;
 }
 
 void CSteamNetworkConnectionBase::PostConnectionStateChangedCallback( ESteamNetworkingConnectionState eOldAPIState, ESteamNetworkingConnectionState eNewAPIState )
@@ -3823,6 +3857,12 @@ CSteamNetworkConnectionPipe::CSteamNetworkConnectionPipe( CSteamNetworkingSocket
 	m_connectionConfig.m_SendRateMin.Set( nRate );
 	m_connectionConfig.m_SendRateMax.Set( nRate );
 
+	// Don't limit the recv buffer.  (Send buffer doesn't
+	// matter since we immediately transfer.)
+	m_connectionConfig.m_RecvBufferSize.Set( 0x10000000 );
+	m_connectionConfig.m_RecvBufferMessages.Set( 0x10000000 );
+	m_connectionConfig.m_RecvMaxMessageSize.Set( 0x10000000 );
+
 	// Diagnostics usually not useful on these types of connections.
 	// (App can enable it or clear this override if it wants to.)
 	#ifdef STEAMNETWORKINGSOCKETS_ENABLE_DIAGNOSTICSUI

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h

@@ -753,7 +753,7 @@ protected:
 
 	/// Called when we receive a complete message.  Should allocate a message object and put it into the proper queues
 	bool ReceivedMessageData( const void *pData, int cbData, int idxLane, int64 nMsgNum, int nFlags, SteamNetworkingMicroseconds usecNow );
-	void ReceivedMessage( CSteamNetworkingMessage *pMsg );
+	bool ReceivedMessage( CSteamNetworkingMessage *pMsg );
 	CSteamNetworkingMessage *AllocateNewRecvMessage( uint32 cbSize, int nFlags, SteamNetworkingMicroseconds usecNow );
 
 	/// Timestamp when we last sent an end-to-end connection request packet

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp

@@ -3369,6 +3369,8 @@ bool CSteamNetworkConnectionBase::SNP_ReceiveReliableSegment( int64 nPktNum, int
 
 	// What do we expect to receive next?
 	const int64 nExpectNextStreamPos = lane.m_nReliableStreamPos + len( lane.m_bufReliableStream );
+	const int32 nMaxRecvBufferSize = m_connectionConfig.m_RecvBufferSize.Get();
+	const int32 nMaxMessageSize = m_connectionConfig.m_RecvMaxMessageSize.Get();
 
 	// Check if we need to grow the reliable buffer to hold the data
 	if ( nSegEnd > nExpectNextStreamPos )
@@ -3381,7 +3383,7 @@ bool CSteamNetworkConnectionBase::SNP_ReceiveReliableSegment( int64 nPktNum, int
 		// against a malicious sender trying to create big gaps.  If they
 		// are legit, they will notice that we go back and fill in the gaps
 		// and we will get caught up.
-		if ( cbNewSize > k_cbMaxBufferedReceiveReliableData )
+		if ( cbNewSize > nMaxRecvBufferSize )
 		{
 			// Stop processing the packet, and don't ack it.
 			// This indicates the connection is in pretty bad shape,
@@ -3650,7 +3652,7 @@ bool CSteamNetworkConnectionBase::SNP_ReceiveReliableSegment( int64 nPktNum, int
 			// Sanity check size.  Note that we do this check before we shift,
 			// to protect against overflow.
 			// (Although DeserializeVarInt doesn't detect overflow...)
-			if ( nMsgSizeUpperBits > (uint64)k_cbMaxMessageSizeRecv<<5 )
+			if ( nMsgSizeUpperBits > (((uint64)nMaxRecvBufferSize)<<5) )
 			{
 				ConnectionState_ProblemDetectedLocally( k_ESteamNetConnectionEnd_Misc_InternalError,
 					"Reliable message size too large.  (%llu<<5 + %d)",
@@ -3660,7 +3662,14 @@ bool CSteamNetworkConnectionBase::SNP_ReceiveReliableSegment( int64 nPktNum, int
 
 			// Compute total size, and check it again
 			cbMsgSize += int( nMsgSizeUpperBits<<5 );
-			if ( cbMsgSize > k_cbMaxMessageSizeRecv )
+			if ( cbMsgSize > nMaxRecvBufferSize )
+			{
+				ConnectionState_ProblemDetectedLocally( k_ESteamNetConnectionEnd_Misc_InternalError,
+					"Reliable message size %d too large.", cbMsgSize );
+				return false;
+			}
+
+			if ( cbMsgSize > nMaxMessageSize )
 			{
 				ConnectionState_ProblemDetectedLocally( k_ESteamNetConnectionEnd_Misc_InternalError,
 					"Reliable message size %d too large.", cbMsgSize );
@@ -3677,7 +3686,10 @@ bool CSteamNetworkConnectionBase::SNP_ReceiveReliableSegment( int64 nPktNum, int
 
 		// We have a full message!  Queue it
 		if ( !ReceivedMessageData( pReliableDecode, cbMsgSize, idxLane, nMsgNum, k_nSteamNetworkingSend_Reliable, usecNow ) )
-			return false; // Weird failure.  Most graceful response is to not ack this packet, and maybe we will work next on retry.
+		{
+			ConnectionState_ProblemDetectedLocally( k_ESteamNetConnectionEnd_Misc_InternalError, "Reliable message size %d too large for remaining space in message queue.", cbMsgSize );
+			return false;
+		}
 		pReliableDecode += cbMsgSize;
 		int cbStreamConsumed = pReliableDecode-pReliableStart;
 

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.h

@@ -33,13 +33,6 @@ constexpr SteamNetworkingMicroseconds k_usecAckDelayPrecision = (1 << k_nAckDela
 // balance between false positive and false negative rates.
 constexpr SteamNetworkingMicroseconds k_usecNackFlush = 3*1000;
 
-// Max size of a message that we are wiling to *receive*.
-constexpr int k_cbMaxMessageSizeRecv = k_cbMaxSteamNetworkingSocketsMessageSizeSend*2;
-
-// The max we will look ahead and allocate data, ahead of the reliable
-// messages we have been able to decode.  We limit this to make sure that
-// a malicious sender cannot exploit us.
-constexpr int k_cbMaxBufferedReceiveReliableData = k_cbMaxMessageSizeRecv + 64*1024;
 constexpr int k_nMaxReliableStreamGaps_Extend = 30; // Discard reliable data past the end of the stream, if it would cause us to get too many gaps
 constexpr int k_nMaxReliableStreamGaps_Fragment = 20; // Discard reliable data that is filling in the middle of a hole, if it would cause the number of gaps to exceed this number
 constexpr int k_nMaxPacketGaps = 62; // Don't bother tracking more than N gaps.  Instead, we will end up NACKing some packets that we actually did receive.  This should not break the protocol, but it protects us from malicious sender
@@ -189,15 +182,21 @@ struct SteamNetworkingMessageQueue
 	CSteamNetworkingMessage *m_pFirst = nullptr;
 	CSteamNetworkingMessage *m_pLast = nullptr;
 	LockDebugInfo *m_pRequiredLock = nullptr; // Is there a lock that is required to be held while we access this queue?
+	int m_nMessageCount = 0;
+	int m_nMessageSize = 0;
+
 
 	inline bool empty() const
 	{
 		if ( m_pFirst )
 		{
 			Assert( m_pLast );
+			Assert( m_nMessageCount > 0 );
 			return false;
 		}
 		Assert( !m_pLast );
+		Assert( m_nMessageCount == 0 );
+		Assert( m_nMessageSize == 0 );
 		return true;
 	}
 
@@ -615,7 +614,7 @@ struct SSNPReceiverState
 
 	/// Queue a flush of ALL acks (and NACKs!) by the given time.
 	/// If anything is scheduled to happen earlier, that schedule
-	/// will still be honered.  We will ack up to that packet number,
+	/// will still be honored.  We will ack up to that packet number,
 	/// and then we we may report higher numbered blocks, or we may
 	/// stop and wait to report more acks until later.
 	void QueueFlushAllAcks( SteamNetworkingMicroseconds usecWhen );
@@ -667,6 +666,7 @@ inline void CSteamNetworkingMessage::LinkBefore( CSteamNetworkingMessage *pSucce
 	Assert( pQueue->m_pFirst );
 	Assert( pQueue->m_pLast );
 	Assert( (pSuccessor->*pMbrLinks).m_pQueue == pQueue );
+	Assert( pQueue->m_nMessageCount > 0 );
 
 	CSteamNetworkingMessage *pPrev = (pSuccessor->*pMbrLinks).m_pPrev;
 	if ( pPrev )
@@ -686,6 +686,11 @@ inline void CSteamNetworkingMessage::LinkBefore( CSteamNetworkingMessage *pSucce
 	}
 
 	// Finish up
+	Assert( pQueue->m_nMessageCount > 0 );
+	++pQueue->m_nMessageCount;
+	Assert( m_cbSize >= 0 );
+	pQueue->m_nMessageSize += m_cbSize;
+
 	(this->*pMbrLinks).m_pQueue = pQueue;
 	(this->*pMbrLinks).m_pNext = pSuccessor;
 	(pSuccessor->*pMbrLinks).m_pPrev = this;
@@ -705,11 +710,13 @@ inline void CSteamNetworkingMessage::LinkToQueueTail( CSteamNetworkingMessage::L
 	{
 		Assert( pQueue->m_pFirst );
 		Assert( !(pQueue->m_pLast->*pMbrLinks).m_pNext );
+		Assert( pQueue->m_nMessageCount > 0 );
 		(pQueue->m_pLast->*pMbrLinks).m_pNext = this;
 	}
 	else
 	{
 		Assert( !pQueue->m_pFirst );
+		Assert( pQueue->m_nMessageCount == 0 );
 		pQueue->m_pFirst = this;
 	}
 
@@ -721,6 +728,9 @@ inline void CSteamNetworkingMessage::LinkToQueueTail( CSteamNetworkingMessage::L
 	pQueue->m_pLast = this;
 
 	// Remember what queue we're in
+	++pQueue->m_nMessageCount;
+	Assert( m_cbSize >= 0 );
+	pQueue->m_nMessageSize += m_cbSize;
 	(this->*pMbrLinks).m_pQueue = pQueue;
 }
 
@@ -737,6 +747,7 @@ inline void CSteamNetworkingMessage::UnlinkFromQueue( CSteamNetworkingMessage::L
 	}
 	SteamNetworkingMessageQueue &q = *links.m_pQueue;
 	q.AssertLockHeld();
+	Assert( q.m_nMessageCount > 0 );
 
 	// Unlink from previous
 	if ( links.m_pPrev )
@@ -763,7 +774,10 @@ inline void CSteamNetworkingMessage::UnlinkFromQueue( CSteamNetworkingMessage::L
 		Assert( q.m_pLast == this );
 		q.m_pLast = links.m_pPrev;
 	}
-
+	--q.m_nMessageCount;
+	Assert( m_cbSize >= 0 );
+	Assert( q.m_nMessageSize >= m_cbSize );
+	q.m_nMessageSize -= m_cbSize;
 	// Clear links
 	links.m_pQueue = nullptr;
 	links.m_pPrev = nullptr;

src/steamnetworkingsockets/steamnetworkingsockets_internal.h

@@ -279,6 +279,10 @@ const int k_cbSteamNetworkingSocketsTypicalMaxPlaintextPayloadSend = k_cbSteamNe
 const int k_cbSteamNetworkingSocketsMaxEncryptedPayloadRecv = k_cbSteamNetworkingSocketsMaxUDPMsgLen;
 const int k_cbSteamNetworkingSocketsMaxPlaintextPayloadRecv = k_cbSteamNetworkingSocketsMaxUDPMsgLen;
 
+/// Max value that RecvMaxMessageSize can be set to.
+const int k_cbMaxMessageSizeRecv_Limit = k_cbMaxSteamNetworkingSocketsMessageSizeSend*2;
+COMPILE_TIME_ASSERT( k_cbMaxMessageSizeRecv_Limit >= k_cbMaxSteamNetworkingSocketsMessageSizeSend*2 );
+
 /// If we have a cert that is going to expire in <N seconds, try to renew it
 const int k_nSecCertExpirySeekRenew = 3600*2;
 
@@ -867,6 +871,9 @@ struct ConnectionConfig
 	ConfigValue<int32> m_TimeoutInitial;
 	ConfigValue<int32> m_TimeoutConnected;
 	ConfigValue<int32> m_SendBufferSize;
+	ConfigValue<int32> m_RecvBufferSize;
+	ConfigValue<int32> m_RecvBufferMessages;
+	ConfigValue<int32> m_RecvMaxMessageSize;
 	ConfigValue<int32> m_SendRateMin;
 	ConfigValue<int32> m_SendRateMax;
 	ConfigValue<int32> m_MTU_PacketSize;