From 1ea859c431b33a384727c0016917dde15bceeff3 Mon Sep 17 00:00:00 2001 From: Magnus Auvinen Date: Mon, 11 Feb 2008 22:25:10 +0000 Subject: security audit: fixed so the packer functions checks for errors --- src/engine/client/ec_client.c | 19 +++++++++++-- src/engine/e_if_msg.h | 15 ++++++++++ src/engine/e_map.c | 2 +- src/engine/e_msg.c | 19 +++++++++++-- src/engine/e_packer.c | 64 +++++++++++++++++++++++++++++++++++++++++-- src/engine/server/es_server.c | 3 ++ 6 files changed, 113 insertions(+), 9 deletions(-) (limited to 'src/engine') 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: + +*/ +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 +#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; -- cgit 1.4.1