Merge pull request #489 from 4144/loginchecks

Add missing checks to login server

4 files changed, 83 insertions, 18 deletions

diff --git a/src/login/account_sql.c b/src/login/account_sql.c
index f745d3d13..8975250b8 100644
--- a/src/login/account_sql.c
+++ b/src/login/account_sql.c
@@ -13,6 +13,7 @@
 #include "../common/console.h"
 #include "../common/malloc.h"
 #include "../common/mmo.h"
+#include "../common/nullpo.h"
 #include "../common/showmsg.h"
 #include "../common/socket.h"
 #include "../common/sql.h"
@@ -136,6 +137,7 @@ static bool account_db_sql_init(AccountDB* self)
 	const char* database;
 	const char* codepage;
 
+	nullpo_ret(db);
 	db->accounts = SQL->Malloc();
 	sql_handle = db->accounts;
 
@@ -181,6 +183,7 @@ static void account_db_sql_destroy(AccountDB* self)
 {
 	AccountDB_SQL* db = (AccountDB_SQL*)self;
 
+	nullpo_retv(db);
 	SQL->Free(db->accounts);
 	db->accounts = NULL;
 	aFree(db);
@@ -192,6 +195,9 @@ static bool account_db_sql_get_property(AccountDB* self, const char* key, char*
 	AccountDB_SQL* db = (AccountDB_SQL*)self;
 	const char* signature;
 
+	nullpo_ret(db);
+	nullpo_ret(key);
+	nullpo_ret(buf);
 	signature = "engine.";
 	if( strncmpi(key, signature, strlen(signature)) == 0 )
 	{
@@ -282,7 +288,9 @@ static bool account_db_sql_set_property(AccountDB* self, const char* key, const
 	AccountDB_SQL* db = (AccountDB_SQL*)self;
 	const char* signature;
 
-
+	nullpo_ret(db);
+	nullpo_ret(key);
+	nullpo_ret(value);
 	signature = "sql.";
 	if( strncmp(key, signature, strlen(signature)) == 0 )
 	{
@@ -360,6 +368,8 @@ static bool account_db_sql_create(AccountDB* self, struct mmo_account* acc)
 
 	// decide on the account id to assign
 	int account_id;
+	nullpo_ret(db);
+	nullpo_ret(acc);
 	if( acc->account_id != -1 )
 	{// caller specifies it manually
 		account_id = acc->account_id;
@@ -410,6 +420,7 @@ static bool account_db_sql_remove(AccountDB* self, const int account_id)
 	Sql* sql_handle = db->accounts;
 	bool result = false;
 
+	nullpo_ret(db);
 	if( SQL_SUCCESS != SQL->QueryStr(sql_handle, "START TRANSACTION")
 	||  SQL_SUCCESS != SQL->Query(sql_handle, "DELETE FROM `%s` WHERE `account_id` = %d", db->account_db, account_id)
 	||  SQL_SUCCESS != SQL->Query(sql_handle, "DELETE FROM `%s` WHERE `account_id` = %d", db->global_acc_reg_num_db, account_id)
@@ -442,11 +453,13 @@ static bool account_db_sql_load_num(AccountDB* self, struct mmo_account* acc, co
 static bool account_db_sql_load_str(AccountDB* self, struct mmo_account* acc, const char* userid)
 {
 	AccountDB_SQL* db = (AccountDB_SQL*)self;
-	Sql* sql_handle = db->accounts;
+	Sql* sql_handle;
 	char esc_userid[2*NAME_LENGTH+1];
 	int account_id;
 	char* data;
 
+	nullpo_ret(db);
+	sql_handle = db->accounts;
 	SQL->EscapeString(sql_handle, esc_userid, userid);
 
 	// get the list of account IDs for this user ID
@@ -481,8 +494,10 @@ static bool account_db_sql_load_str(AccountDB* self, struct mmo_account* acc, co
 static AccountDBIterator* account_db_sql_iterator(AccountDB* self)
 {
 	AccountDB_SQL* db = (AccountDB_SQL*)self;
-	AccountDBIterator_SQL* iter = (AccountDBIterator_SQL*)aCalloc(1, sizeof(AccountDBIterator_SQL));
+	AccountDBIterator_SQL* iter;
 
+	nullpo_retr(NULL, db);
+	iter = (AccountDBIterator_SQL*)aCalloc(1, sizeof(AccountDBIterator_SQL));
 	// set up the vtable
 	iter->vtable.destroy = &account_db_sql_iter_destroy;
 	iter->vtable.next    = &account_db_sql_iter_next;
@@ -507,10 +522,14 @@ static void account_db_sql_iter_destroy(AccountDBIterator* self)
 static bool account_db_sql_iter_next(AccountDBIterator* self, struct mmo_account* acc)
 {
 	AccountDBIterator_SQL* iter = (AccountDBIterator_SQL*)self;
-	AccountDB_SQL* db = (AccountDB_SQL*)iter->db;
-	Sql* sql_handle = db->accounts;
+	AccountDB_SQL* db;
+	Sql* sql_handle;
 	char* data;
 
+	nullpo_ret(iter);
+	db = (AccountDB_SQL*)iter->db;
+	nullpo_ret(db);
+	sql_handle = db->accounts;
 	// get next account ID
 	if( SQL_ERROR == SQL->Query(sql_handle, "SELECT `account_id` FROM `%s` WHERE `account_id` > '%d' ORDER BY `account_id` ASC LIMIT 1",
 		db->account_db, iter->last_account_id) )
@@ -539,9 +558,12 @@ static bool account_db_sql_iter_next(AccountDBIterator* self, struct mmo_account
 
 static bool mmo_auth_fromsql(AccountDB_SQL* db, struct mmo_account* acc, int account_id)
 {
-	Sql* sql_handle = db->accounts;
+	Sql* sql_handle;
 	char* data;
 
+	nullpo_ret(db);
+	nullpo_ret(acc);
+	sql_handle = db->accounts;
 	// retrieve login entry for the specified account
 	if( SQL_ERROR == SQL->Query(sql_handle,
 	    "SELECT `account_id`,`userid`,`user_pass`,`sex`,`email`,`group_id`,`state`,`unban_time`,`expiration_time`,`logincount`,`lastlogin`,`last_ip`,`birthdate`,`character_slots`,`pincode`,`pincode_change` FROM `%s` WHERE `account_id` = %d",
@@ -581,10 +603,14 @@ static bool mmo_auth_fromsql(AccountDB_SQL* db, struct mmo_account* acc, int acc
 
 static bool mmo_auth_tosql(AccountDB_SQL* db, const struct mmo_account* acc, bool is_new)
 {
-	Sql* sql_handle = db->accounts;
-	SqlStmt* stmt = SQL->StmtMalloc(sql_handle);
+	Sql* sql_handle;
+	SqlStmt* stmt;
 	bool result = false;
 
+	nullpo_ret(db);
+	sql_handle = db->accounts;
+	stmt = SQL->StmtMalloc(sql_handle);
+
 	// try
 	do
 	{
@@ -666,6 +692,7 @@ void mmo_save_accreg2(AccountDB* self, int fd, int account_id, int char_id) {
 	AccountDB_SQL* db = (AccountDB_SQL*)self;
 	int count = RFIFOW(fd, 12);
 
+	nullpo_retv(db);
 	if (count) {
 		int cursor = 14, i;
 		char key[32], sval[254];
@@ -709,12 +736,14 @@ void mmo_save_accreg2(AccountDB* self, int fd, int account_id, int char_id) {
 }
 
 void mmo_send_accreg2(AccountDB* self, int fd, int account_id, int char_id) {
-	Sql* sql_handle = ((AccountDB_SQL*)self)->accounts;
+	Sql* sql_handle;
 	AccountDB_SQL* db = (AccountDB_SQL*)self;
 	char* data;
 	int plen = 0;
 	size_t len;
 
+	nullpo_retv(db);
+	sql_handle = ((AccountDB_SQL*)self)->accounts;
 	if( SQL_ERROR == SQL->Query(sql_handle, "SELECT `key`, `index`, `value` FROM `%s` WHERE `account_id`='%d'", db->global_acc_reg_str_db, account_id) )
 		Sql_ShowDebug(sql_handle);
 
diff --git a/src/login/ipban_sql.c b/src/login/ipban_sql.c
index bfc90ad11..f995450ce 100644
--- a/src/login/ipban_sql.c
+++ b/src/login/ipban_sql.c
@@ -14,6 +14,7 @@
 #include "../common/cbasetypes.h"
 #include "../common/db.h"
 #include "../common/malloc.h"
+#include "../common/nullpo.h"
 #include "../common/sql.h"
 #include "../common/socket.h"
 #include "../common/strlib.h"
@@ -118,6 +119,8 @@ bool ipban_config_read(const char* key, const char* value)
 {
 	const char* signature;
 
+	nullpo_ret(key);
+	nullpo_ret(value);
 	if( ipban_inited )
 		return false;// settings can only be changed before init
 
@@ -220,8 +223,8 @@ bool ipban_check(uint32 ip)
 		return true;
 	}
 
-	if( SQL_ERROR == SQL->NextRow(sql_handle) )
-		return true;// Shouldn't happen, but just in case...
+	if( SQL_SUCCESS != SQL->NextRow(sql_handle) )
+		return false;
 
 	SQL->GetData(sql_handle, 0, &data, NULL);
 	matches = atoi(data);
@@ -244,9 +247,11 @@ void ipban_log(uint32 ip)
 	if( failures >= login_config.dynamic_pass_failure_ban_limit )
 	{
 		uint8* p = (uint8*)&ip;
-		if( SQL_ERROR == SQL->Query(sql_handle, "INSERT INTO `%s`(`list`,`btime`,`rtime`,`reason`) VALUES ('%u.%u.%u.*', NOW() , NOW() +  INTERVAL %d MINUTE ,'Password error ban')",
-			ipban_table, p[3], p[2], p[1], login_config.dynamic_pass_failure_ban_duration) )
+		if (SQL_ERROR == SQL->Query(sql_handle, "INSERT INTO `%s`(`list`,`btime`,`rtime`,`reason`) VALUES ('%u.%u.%u.*', NOW() , NOW() +  INTERVAL %d MINUTE ,'Password error ban')",
+			ipban_table, p[3], p[2], p[1], login_config.dynamic_pass_failure_ban_duration))
+		{
 			Sql_ShowDebug(sql_handle);
+		}
 	}
 }
 
diff --git a/src/login/login.c b/src/login/login.c
index 846f24027..95cbab70d 100644
--- a/src/login/login.c
+++ b/src/login/login.c
@@ -19,6 +19,7 @@
 #include "../common/db.h"
 #include "../common/malloc.h"
 #include "../common/md5calc.h"
+#include "../common/nullpo.h"
 #include "../common/random.h"
 #include "../common/showmsg.h"
 #include "../common/socket.h"
@@ -98,6 +99,7 @@ static int login_online_db_setoffline(DBKey key, DBData *data, va_list ap)
 {
 	struct online_login_data* p = DB->data2ptr(data);
 	int server_id = va_arg(ap, int);
+	nullpo_ret(p);
 	if( server_id == -1 )
 	{
 		p->char_server = -1;
@@ -118,6 +120,7 @@ static int login_online_db_setoffline(DBKey key, DBData *data, va_list ap)
 static int login_online_data_cleanup_sub(DBKey key, DBData *data, va_list ap)
 {
 	struct online_login_data *character= DB->data2ptr(data);
+	nullpo_ret(character);
 	if (character->char_server == -2) //Unknown server.. set them offline
 		login->remove_online_user(character->account_id);
 	return 0;
@@ -136,6 +139,7 @@ int charif_sendallwos(int sfd, uint8* buf, size_t len)
 {
 	int i, c;
 
+	nullpo_ret(buf);
 	for( i = 0, c = 0; i < ARRAYLENGTH(server); ++i )
 	{
 		int fd = server[i].fd;
@@ -155,6 +159,7 @@ int charif_sendallwos(int sfd, uint8* buf, size_t len)
 /// Initializes a server structure.
 void chrif_server_init(int id)
 {
+	Assert_retv(id >= 0 && id < MAX_SERVERS);
 	memset(&server[id], 0, sizeof(server[id]));
 	server[id].fd = -1;
 }
@@ -163,7 +168,8 @@ void chrif_server_init(int id)
 /// Destroys a server structure.
 void chrif_server_destroy(int id)
 {
-	if( server[id].fd != -1 )
+	Assert_retv(id >= 0 && id < MAX_SERVERS);
+	if (server[id].fd != -1)
 	{
 		do_close(server[id].fd);
 		server[id].fd = -1;
@@ -183,6 +189,7 @@ void chrif_server_reset(int id)
 /// Called when the connection to Char Server is disconnected.
 void chrif_on_disconnect(int id)
 {
+	Assert_retv(id >= 0 && id < MAX_SERVERS);
 	ShowStatus("Char-server '%s' has disconnected.\n", server[id].name);
 	chrif_server_reset(id);
 }
@@ -207,6 +214,9 @@ bool login_check_encrypted(const char* str1, const char* str2, const char* passw
 {
 	char tmpstr[64+1], md5str[32+1];
 
+	nullpo_ret(str1);
+	nullpo_ret(str2);
+	nullpo_ret(passwd);
 	safesnprintf(tmpstr, sizeof(tmpstr), "%s%s", str1, str2);
 	MD5_String(tmpstr, md5str);
 
@@ -215,6 +225,8 @@ bool login_check_encrypted(const char* str1, const char* str2, const char* passw
 
 bool login_check_password(const char* md5key, int passwdenc, const char* passwd, const char* refpass)
 {
+	nullpo_ret(passwd);
+	nullpo_ret(refpass);
 	if(passwdenc == 0)
 	{
 		return (0==strcmp(passwd, refpass));
@@ -248,6 +260,7 @@ int login_lan_config_read(const char *lancfgName)
 	int line_num = 0;
 	char line[1024], w1[64], w2[64], w3[64], w4[64];
 
+	nullpo_ret(lancfgName);
 	if((fp = fopen(lancfgName, "r")) == NULL) {
 		ShowWarning("LAN Support configuration file is not found: %s\n", lancfgName);
 		return 1;
@@ -348,6 +361,7 @@ void login_fromchar_parse_auth(int fd, int id, const char *const ip)
 	}
 	else
 	{// authentication not found
+		nullpo_retv(ip);
 		ShowStatus("Char-server '%s': authentication of the account %d REFUSED (ip: %s).\n", server[id].name, account_id, ip);
 		login->fromchar_auth_ack(fd, account_id, login_id1, login_id2, sex, request_id, NULL);
 	}
@@ -1008,6 +1022,9 @@ int login_mmo_auth_new(const char* userid, const char* pass, const char sex, con
 	int64 tick = timer->gettick();
 	struct mmo_account acc;
 
+	nullpo_retr(3, userid);
+	nullpo_retr(3, pass);
+	nullpo_retr(3, last_ip);
 	//Account Registration Flood Protection by [Kevin]
 	if( new_reg_tick == 0 )
 		new_reg_tick = timer->gettick();
@@ -1065,6 +1082,7 @@ int login_mmo_auth(struct login_session_data* sd, bool isServer) {
 	size_t len;
 
 	char ip[16];
+	nullpo_ret(sd);
 	ip2str(session[sd->fd]->client_addr, ip);
 
 	// DNS Blacklist check
@@ -1206,6 +1224,7 @@ void login_connection_problem(int fd, uint8 status)
 void login_kick(struct login_session_data* sd)
 {
 	uint8 buf[6];
+	nullpo_retv(sd);
 	WBUFW(buf,0) = 0x2734;
 	WBUFL(buf,2) = sd->account_id;
 	charif_sendallwos(-1, buf, 6);
@@ -1214,13 +1233,15 @@ void login_kick(struct login_session_data* sd)
 void login_auth_ok(struct login_session_data* sd)
 {
 	int fd = sd->fd;
-	uint32 ip = session[fd]->client_addr;
+	uint32 ip;
 
 	uint8 server_num, n;
 	uint32 subnet_char_ip;
 	struct login_auth_node* node;
 	int i;
 
+	nullpo_retv(sd);
+	ip = session[fd]->client_addr;
 	if( runflag != LOGINSERVER_ST_RUNNING )
 	{
 		// players can only login while running
@@ -1336,9 +1357,12 @@ void login_auth_ok(struct login_session_data* sd)
 
 void login_auth_failed(struct login_session_data* sd, int result)
 {
-	int fd = sd->fd;
-	uint32 ip = session[fd]->client_addr;
+	int fd;
+	uint32 ip;
+	nullpo_retv(sd);
 
+	fd = sd->fd;
+	ip = session[fd]->client_addr;
 	if (login_config.log_login)
 	{
 		const char* error;
@@ -1738,7 +1762,9 @@ void login_set_defaults()
 int login_config_read(const char* cfgName)
 {
 	char line[1024], w1[1024], w2[1024];
-	FILE* fp = fopen(cfgName, "r");
+	FILE* fp;
+	nullpo_retr(1, cfgName);
+	fp = fopen(cfgName, "r");
 	if (fp == NULL) {
 		ShowError("Configuration file (%s) not found.\n", cfgName);
 		return 1;
diff --git a/src/login/loginlog_sql.c b/src/login/loginlog_sql.c
index 5654b4c5b..8ed6e395a 100644
--- a/src/login/loginlog_sql.c
+++ b/src/login/loginlog_sql.c
@@ -11,6 +11,7 @@
 
 #include "../common/cbasetypes.h"
 #include "../common/mmo.h"
+#include "../common/nullpo.h"
 #include "../common/socket.h"
 #include "../common/sql.h"
 #include "../common/strlib.h"
@@ -67,6 +68,8 @@ void login_log(uint32 ip, const char* username, int rcode, const char* message)
 	char esc_message[255*2+1];
 	int retcode;
 
+	nullpo_retv(username);
+	nullpo_retv(message);
 	if( !enabled )
 		return;
 
@@ -137,6 +140,8 @@ bool loginlog_config_read(const char* key, const char* value)
 {
 	const char* signature;
 
+	nullpo_ret(key);
+	nullpo_ret(value);
 	signature = "sql.";
 	if( strncmpi(key, signature, strlen(signature)) == 0 )
 	{