tier1: fix possible unsafe read in lzss SafeUncompress, make tests pass under asan

Author: nillerusr

engine/common.cpp

@@ -1385,7 +1385,7 @@ bool COM_BufferToBufferDecompress( void *dest, unsigned int *destLen, const void
 		if ( pHeader->id == LZSS_ID )
 		{
 			CLZSS s;
-			int nActualDecompressedSize = s.SafeUncompress( (byte *)source, (byte *)dest, *destLen );
+			int nActualDecompressedSize = s.SafeUncompress( (byte *)source, sourceLen, (byte *)dest, *destLen );
 			if ( nActualDecompressedSize != nDecompressedSize )
 			{
 				Warning( "NET_BufferToBufferDecompress: header said %d bytes would be decompressed, but we LZSS decompressed %d\n", nDecompressedSize, nActualDecompressedSize );

gameui/BasePanel.cpp

@@ -915,7 +915,7 @@ CBasePanel::CBasePanel() : Panel(NULL, "BaseGameUIPanel")
 		}
 	}
 
-//	if( IsAndroid() )
+	if( IsAndroid() )
 	{
 		AddUrlButton( this, "vgui/\x64\x69\x73\x63\x6f\x72\x64\x5f\x6c\x6f\x67\x6f", "\x68\x74\x74\x70\x73\x3a\x2f\x2f\x64\x69\x73\x63\x6f\x72\x64\x2e\x67\x67\x2f\x68\x5a\x52\x42\x37\x57\x4d\x67\x47\x77" );
 		AddUrlButton( this, "vgui/\x74\x77\x69\x74\x74\x65\x72\x5f\x6c\x6f\x67\x6f", "\x68\x74\x74\x70\x73\x3a\x2f\x2f\x74\x77\x69\x74\x74\x65\x72\x2e\x63\x6f\x6d\x2f\x6e\x69\x6c\x6c\x65\x72\x75\x73\x72" );

public/tier0/platform.h

@@ -606,6 +606,16 @@ typedef void * HINSTANCE;
 	#define FMTFUNCTION( a, b )
 #endif
 
+#if defined(__has_feature)
+#if __has_feature(address_sanitizer)
+#define USING_ASAN 1
+#endif
+#endif
+
+#if !defined( USING_ASAN ) && defined( __SANITIZE_ADDRESS__ )
+#define USING_ASAN 1
+#endif
+
 #if COMPILER_CLANG || COMPILER_GCC
 #define NO_ASAN __attribute__((no_sanitize("address")))
 #else

public/tier1/lzss.h

@@ -30,7 +30,7 @@ class CLZSS
 	unsigned char*	CompressNoAlloc( const unsigned char *pInput, int inputlen, unsigned char *pOutput, unsigned int *pOutputSize );
 	unsigned int	Uncompress( const unsigned char *pInput, unsigned char *pOutput );
 	//unsigned int	Uncompress( unsigned char *pInput, CUtlBuffer &buf );
-	unsigned int	SafeUncompress( const unsigned char *pInput, unsigned char *pOutput, unsigned int unBufSize );
+	unsigned int	SafeUncompress( const unsigned char *pInput, unsigned int inputSize, unsigned char *pOutput, unsigned int unBufSize );
 
 	static bool			IsCompressed( const unsigned char *pInput );
 	static unsigned int	GetActualSize( const unsigned char *pInput );

tier1/lzss.cpp

@@ -294,59 +294,61 @@ unsigned int CLZSS::Uncompress( unsigned char *pInput, CUtlBuffer &buf )
 }
 */
 
-unsigned int CLZSS::SafeUncompress( const unsigned char *pInput, unsigned char *pOutput, unsigned int unBufSize )
+unsigned int CLZSS::SafeUncompress( const unsigned char *pInput, unsigned int inputSize, unsigned char *pOutput, unsigned int unBufSize )
 {
 	unsigned int totalBytes = 0;
 	int cmdByte = 0;
 	int getCmdByte = 0;
 
 	unsigned int actualSize = GetActualSize( pInput );
-	if ( !actualSize )
-	{
-		// unrecognized
-		return 0;
-	}
 
-	if ( actualSize > unBufSize )
-	{
+	if ( !actualSize ||
+		actualSize > unBufSize ||
+		inputSize <= sizeof( lzss_header_t ) )
 		return 0;
-	}
+
+	const unsigned char *pInputEnd = pInput+inputSize-1;
+	const unsigned char *pOrigOutput = pOutput;
 
 	pInput += sizeof( lzss_header_t );
 
 	for ( ;; )
 	{
-		if ( !getCmdByte ) 
+		if ( !getCmdByte )
 		{
+			if( pInput > pInputEnd )
+				return 0;
+
 			cmdByte = *pInput++;
 		}
 		getCmdByte = ( getCmdByte + 1 ) & 0x07;
 
 		if ( cmdByte & 0x01 )
 		{
+			if( pInput+1 > pInputEnd )
+				return 0;
+
 			int position = *pInput++ << LZSS_LOOKSHIFT;
 			position |= ( *pInput >> LZSS_LOOKSHIFT );
 			int count = ( *pInput++ & 0x0F ) + 1;
-			if ( count == 1 ) 
-			{
+			if ( count == 1 )
 				break;
-			}
+
 			unsigned char *pSource = pOutput - position - 1;
 
-			if ( totalBytes + count > unBufSize )
-			{
+			if ( totalBytes + count > unBufSize ||
+				pSource < pOrigOutput )
 				return 0;
-			}
 
 			for ( int i=0; i<count; i++ )
-			{
 				*pOutput++ = *pSource++;
-			}
+
 			totalBytes += count;
-		} 
-		else 
+		}
+		else
 		{
-			if ( totalBytes + 1 > unBufSize )
+			if ( totalBytes + 1 > unBufSize ||
+				pInput > pInputEnd )
 				return 0;
 
 			*pOutput++ = *pInput++;

unittests/tier1test/lzsstest.cpp

@@ -0,0 +1,78 @@
+#include "tier0/dbg.h"
+#include "unitlib/unitlib.h"
+#include "tier1/lzss.h"
+
+#ifdef USING_ASAN
+#include <sanitizer/asan_interface.h>
+#endif
+
+DEFINE_TESTSUITE( LZSSSafeUncompressTestSuite )
+
+static void SafeUncompressTests()
+{
+	CLZSS lzss;
+	char poision1[8192];
+	unsigned char in[256];
+
+	char poision2[8192];
+
+	unsigned char out[256];
+	char poision3[8192];
+
+#ifdef USING_ASAN
+	ASAN_POISON_MEMORY_REGION( poision1, 8192 );
+	ASAN_POISON_MEMORY_REGION( poision2, 8192 );
+	ASAN_POISON_MEMORY_REGION( poision3, 8192 );
+#endif
+
+	lzss_header_t *pHeader = (lzss_header_t*)in;
+	pHeader->actualSize = sizeof(in)-sizeof(lzss_header_t);
+	pHeader->id = LZSS_ID;
+
+	uint result = 0;
+
+	// 0xff bytes test
+	memset(in+8, 0xFF, sizeof(in)-sizeof(lzss_header_t));
+	result = lzss.SafeUncompress( in, sizeof(in), out, sizeof(out) );
+	Shipping_Assert( result == 0 );
+
+	// zero bytes test
+	memset(in+8, 0x0, sizeof(in)-sizeof(lzss_header_t));
+	result = lzss.SafeUncompress( in, sizeof(in), out, sizeof(out) );
+	Shipping_Assert( result == 0 );
+
+	// valid data, invalid header
+	pHeader->actualSize = 1;
+	pHeader->id = LZSS_ID;
+	result = lzss.SafeUncompress( in, sizeof(in), out, sizeof(out) );
+	Shipping_Assert( result == 0 );
+
+	pHeader->actualSize = 999;
+	pHeader->id = LZSS_ID;
+	result = lzss.SafeUncompress( in, sizeof(in), out, sizeof(out) );
+	Shipping_Assert( result == 0 );
+
+	pHeader->actualSize = 999;
+	pHeader->id = 1337;
+	result = lzss.SafeUncompress( in, sizeof(in), out, sizeof(out) );
+	Shipping_Assert( result == 0 );
+
+
+	// valid header, valid data
+	const unsigned char compressed[] = {0x4c,0x5a,0x53,0x53,0x1a,0x0,0x0,0x0,0x0,0x44,0x6f,0x20,0x79,0x6f,0x75,0x20,0x6c,0x0,0x69,0x6b,0x65,0x20,0x77,0x68,0x61,0x74,0x41,0x0,0xd4,0x73,0x65,0x65,0x3f,0x0,0x0,0x0};
+
+	pHeader->actualSize = sizeof(compressed);
+	pHeader->id = LZSS_ID;
+	result = lzss.SafeUncompress( compressed, sizeof(compressed), out, sizeof(out) );
+
+	const char data[] = "Do you like what you see?";
+	Shipping_Assert( memcmp(out, data, 26) == 0 );
+	Shipping_Assert( result == 26 );
+}
+
+DEFINE_TESTCASE( LZSSSafeUncompressTest, LZSSSafeUncompressTestSuite )
+{
+	Msg( "Running CLZSS::SafeUncompress tests\n" );
+
+	SafeUncompressTests();
+}

unittests/tier1test/wscript

@@ -14,7 +14,7 @@ def configure(conf):
 	conf.define('TIER1TEST_EXPORTS', 1)
 
 def build(bld):
-	source = ['commandbuffertest.cpp', 'utlstringtest.cpp', 'tier1test.cpp'] #, 'lzsstest.cpp']
+	source = ['commandbuffertest.cpp', 'utlstringtest.cpp', 'tier1test.cpp', 'lzsstest.cpp']
 	includes = ['../../public', '../../public/tier0']
 	defines = []
 	libs = ['tier0', 'tier1', 'mathlib', 'unitlib']