security audit: fixed so the packer functions checks for errors

8 files changed, 116 insertions, 12 deletions

diff --git a/default.bam b/default.bam
index 42fadfed..edba03ab 100644
--- a/default.bam
+++ b/default.bam
@@ -173,7 +173,7 @@ function build(settings)
 	if family == "windows" then
 		settings.cc.flags = "/wd4244"
 	else
-		settings.cc.flags = "-Wall"
+		settings.cc.flags = "-Wall -fstack-protector -fstack-protector-all"
 		settings.linker.flags = ""
 	end
 
diff --git a/src/engine/client/ec_client.c b/src/engine/client/ec_client.c
index 4d83354b..7185c471 100644
--- a/src/engine/client/ec_client.c
+++ b/src/engine/client/ec_client.c
@@ -262,6 +262,10 @@ int client_send_msg()
 {
 	const MSG_INFO *info = msg_get_info();
 	NETPACKET packet;
+	
+	if(!info)
+		return -1;
+		
 	mem_zero(&packet, sizeof(NETPACKET));
 	
 	packet.client_id = 0;
@@ -687,7 +691,8 @@ static void client_process_packet(NETPACKET *packet)
 				}
 				
 				/* TODO: unpack players aswell */
-				client_serverbrowse_set(&packet->address, 0, &info);
+				if(!up.error)
+					client_serverbrowse_set(&packet->address, 0, &info);
 			}
 		}
 	}
@@ -705,6 +710,9 @@ static void client_process_packet(NETPACKET *packet)
 				int map_crc = msg_unpack_int();
 				const char *error = 0;
 				int i;
+
+				if(msg_unpack_error())
+					return;
 				
 				for(i = 0; map[i]; i++) /* protect the player from nasty map names */
 				{
@@ -813,6 +821,7 @@ static void client_process_packet(NETPACKET *packet)
 				int part_size = 0;
 				int crc = 0;
 				int complete_size = 0;
+				const char *data = 0;
 				
 				if(msg == NETMSG_SNAP)
 				{
@@ -825,6 +834,11 @@ static void client_process_packet(NETPACKET *packet)
 					crc = msg_unpack_int();
 					part_size = msg_unpack_int();
 				}
+
+				data = (const char *)msg_unpack_raw(part_size);
+				
+				if(msg_unpack_error())
+					return;
 				
 				/* TODO: adjust our prediction time */
 				if(time_left)
@@ -851,8 +865,7 @@ static void client_process_packet(NETPACKET *packet)
 				if(snapshot_part == part && game_tick > current_recv_tick)
 				{
 					/* TODO: clean this up abit */
-					const char *d = (const char *)msg_unpack_raw(part_size);
-					mem_copy((char*)snapshot_incomming_data + part*MAX_SNAPSHOT_PACKSIZE, d, part_size);
+					mem_copy((char*)snapshot_incomming_data + part*MAX_SNAPSHOT_PACKSIZE, data, part_size);
 					snapshot_part++;
 				
 					if(snapshot_part == num_parts)
diff --git a/src/engine/e_if_msg.h b/src/engine/e_if_msg.h
index 11360b43..d03a64a6 100644
--- a/src/engine/e_if_msg.h
+++ b/src/engine/e_if_msg.h
@@ -133,4 +133,19 @@ const char *msg_unpack_string();
 */
 const unsigned char *msg_unpack_raw(int size);
 
+/*
+	Function: msg_unpack_error
+		TODO
+	
+	Arguments:
+		arg1 - desc
+	
+	Returns:
+
+	See Also:
+		<other_func>
+*/
+int msg_unpack_error();
+
+
 #endif
diff --git a/src/engine/e_map.c b/src/engine/e_map.c
index c689be45..4240225a 100644
--- a/src/engine/e_map.c
+++ b/src/engine/e_map.c
@@ -1,5 +1,5 @@
 /* copyright (c) 2007 magnus auvinen, see licence.txt for more info */
-#include <stdio.h>
+#include "e_system.h"
 #include "e_datafile.h"
 
 static DATAFILE *map = 0;
diff --git a/src/engine/e_msg.c b/src/engine/e_msg.c
index 0bdaf856..f9efc2bf 100644
--- a/src/engine/e_msg.c
+++ b/src/engine/e_msg.c
@@ -5,6 +5,7 @@
 /* message packing */
 static PACKER msg_packer;
 static MSG_INFO pack_info;
+static int packer_failed = 0;
 
 void msg_pack_int(int i) { packer_add_int(&msg_packer, i); }
 void msg_pack_string(const char *p, int limit) { packer_add_string(&msg_packer, p, limit); }
@@ -15,6 +16,7 @@ void msg_pack_start_system(int msg, int flags)
 	packer_reset(&msg_packer);
 	pack_info.msg = (msg<<1)|1;
 	pack_info.flags = flags;
+	packer_failed = 0;
 	
 	msg_pack_int(pack_info.msg);
 }
@@ -24,18 +26,30 @@ void msg_pack_start(int msg, int flags)
 	packer_reset(&msg_packer);
 	pack_info.msg = msg<<1;
 	pack_info.flags = flags;
+	packer_failed = 0;
 	
 	msg_pack_int(pack_info.msg);
 }
 
 void msg_pack_end()
 {
-	pack_info.size = packer_size(&msg_packer);
-	pack_info.data = packer_data(&msg_packer);
+	if(msg_packer.error)
+	{
+		packer_failed = 1;
+		pack_info.size = 0;
+		pack_info.data = (unsigned char *)"";
+	}
+	else
+	{
+		pack_info.size = packer_size(&msg_packer);
+		pack_info.data = packer_data(&msg_packer);
+	}
 }
 
 const MSG_INFO *msg_get_info()
 {
+	if(packer_failed)
+		return 0;
 	return &pack_info;
 }
 
@@ -53,3 +67,4 @@ int msg_unpack_start(const void *data, int data_size, int *system)
 int msg_unpack_int() { return unpacker_get_int(&msg_unpacker); }
 const char *msg_unpack_string() { return unpacker_get_string(&msg_unpacker); }
 const unsigned char *msg_unpack_raw(int size)  { return unpacker_get_raw(&msg_unpacker, size); }
+int msg_unpack_error() { return msg_unpacker.error; }
diff --git a/src/engine/e_packer.c b/src/engine/e_packer.c
index 948db4ea..9e77927d 100644
--- a/src/engine/e_packer.c
+++ b/src/engine/e_packer.c
@@ -1,8 +1,13 @@
 /* copyright (c) 2007 magnus auvinen, see licence.txt for more info */
 
+#include "e_system.h"
 #include "e_packer.h"
 #include "e_compression.h"
 
+/* useful for debugging */
+#define packing_error(p) p->error = 1
+/* #define packing_error(p) p->error = 1; dbg_break() */
+
 void packer_reset(PACKER *p)
 {
 	p->error = 0;
@@ -12,30 +17,66 @@ void packer_reset(PACKER *p)
 
 void packer_add_int(PACKER *p, int i)
 {
-	p->current = vint_pack(p->current, i);
+	if(p->error)
+		return;
+	
+	/* make sure that we have space enough */
+	if(p->end - p->current < 6)
+	{
+		dbg_break();
+		p->error = 1;
+	}
+	else
+		p->current = vint_pack(p->current, i);
 }
 
 void packer_add_string(PACKER *p, const char *str, int limit)
 {
+	if(p->error)
+		return;
+		
 	if(limit > 0)
 	{
 		while(*str && limit != 0)
 		{
 			*p->current++ = *str++;
 			limit--;
+			
+			if(p->current >= p->end)
+			{
+				packing_error(p);
+				break;
+			}
 		}
 		*p->current++ = 0;
 	}
 	else
 	{
 		while(*str)
+		{
 			*p->current++ = *str++;
+
+			if(p->current >= p->end)
+			{
+				packing_error(p);
+				break;
+			}
+		}
 		*p->current++ = 0;
 	}
 }
 
 void packer_add_raw(PACKER *p, const unsigned char *data, int size)
 {
+	if(p->error)
+		return;
+		
+	if(p->current+size >= p->end)
+	{
+		packing_error(p);
+		return;
+	}
+	
 	while(size)
 	{
 		*p->current++ = *data++;
@@ -64,21 +105,33 @@ void unpacker_reset(UNPACKER *p, const unsigned char *data, int size)
 int unpacker_get_int(UNPACKER *p)
 {
 	int i;
-	if(p->current >= p->end)
+	if(p->error || p->current >= p->end)
 		return 0;
 	p->current = vint_unpack(p->current, &i);
+	if(p->current > p->end)
+	{
+		packing_error(p);
+		return 0;
+	}
 	return i;
 }
 
 const char *unpacker_get_string(UNPACKER *p)
 {
 	const char *ptr;
-	if(p->current >= p->end)
+	if(p->error || p->current >= p->end)
 		return "";
 		
 	ptr = (const char *)p->current;
 	while(*p->current) /* skip the string */
+	{
 		p->current++;
+		if(p->current == p->end)
+		{
+			packing_error(p);
+			return "";
+		}
+	}
 	p->current++;
 	return ptr;
 }
@@ -87,5 +140,10 @@ const unsigned char *unpacker_get_raw(UNPACKER *p, int size)
 {
 	const unsigned char *ptr = p->current;
 	p->current += size;
+	if(p->current > p->end)
+	{
+		packing_error(p);
+		return 0;
+	}
 	return ptr;
 }
diff --git a/src/engine/server/es_server.c b/src/engine/server/es_server.c
index 7bfb7a20..d8b8d978 100644
--- a/src/engine/server/es_server.c
+++ b/src/engine/server/es_server.c
@@ -284,6 +284,9 @@ int server_send_msg(int client_id)
 {
 	const MSG_INFO *info = msg_get_info();
 	NETPACKET packet;
+	if(!info)
+		return -1;
+		
 	mem_zero(&packet, sizeof(NETPACKET));
 	
 	packet.client_id = client_id;
diff --git a/src/game/client/gc_client.cpp b/src/game/client/gc_client.cpp
index b8ffac09..20d1df7c 100644
--- a/src/game/client/gc_client.cpp
+++ b/src/game/client/gc_client.cpp
@@ -546,7 +546,7 @@ void render_spectators(float x, float y, float w)
 	int count = 0;
 	float h = 120.0f;
 	
-	str_copy(buffer, sizeof(buffer), "Spectators: ");
+	str_copy(buffer, "Spectators: ", sizeof(buffer));
 
 	gfx_blend_normal();
 	gfx_texture_set(-1);
@@ -614,7 +614,7 @@ void render_scoreboard(float x, float y, float w, int team, const char *title)
 		if(gameobj)
 		{
 			char buf[128];
-			str_format(buf, buf, "%d", gameobj->teamscore[team&1]);
+			str_format(buf, sizeof(buf), "%d", gameobj->teamscore[team&1]);
 			tw = gfx_text_width(0, 48, buf, -1);
 			gfx_text(0, x+w-tw-30, y, 48, buf, -1);
 		}