Fix #2425 Character variable lengths and char-server (#2647)

* Fix #2425 Character variable lengths and char-server

Medium term fox for crash on char-serv caused by too long character variable.
Add few error msg and safety.
Add npc/test/npc_test_longvar.txt as a basic test.

NB:
The config.in for max_colum size if not really set as best would either retrive that value from sql, or use the same config file to actually set the sql column size...
May be handle later with some kind of hibernate ORM.

Thanks to @Tokeiburu, @aleos89, @Lemongrass3110.

npc/test/npc_test_longvar.txt

@@ -0,0 +1,23 @@
+//===== rAthena Script =======================================
+//= Sample: Character variable lengths Test
+//===== By: ==================================================
+//= rAthena Dev Team
+//===== Last Updated: ========================================
+//= 20171125
+//===== Description: ========================================= 
+//= An validation test about long character variable stored on char-serv
+//= from https://github.com/rathena/rathena/issues/2425
+//============================================================
+
+prontera,155,176,3	script	Pront Test	77,{
+	mes "Testing to store long variable on char-serv";
+//test1 long value
+	test$ = "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345";
+//test2 normal length
+	test2$ = "anything really, doesn't matter at this point";
+//test3 long key (41)
+        thisisareallylongkeyImeanReallyReallyLong$ = "gotcha";
+//test4 long key 2 (with int)
+	ReallyReallyReallyReallyReallyReallyLong = 42;
+	end;
+}

src/char/inter.cpp

@@ -1272,37 +1272,44 @@ int mapif_parse_Registry(int fd)
 
 	if( count ) {
 		int cursor = 14, i;
-		char key[32], sval[254];
 		bool isLoginActive = session_isActive(login_fd);
 
 		if( isLoginActive )
 			chlogif_upd_global_accreg(account_id,char_id);
 
 		for(i = 0; i < count; i++) {
-			unsigned int index;
-			safestrncpy(key, RFIFOCP(fd, cursor + 1), RFIFOB(fd, cursor));
-			cursor += RFIFOB(fd, cursor) + 1;
+			size_t lenkey = RFIFOB( fd, cursor );
+			const char* src_key= RFIFOCP(fd, cursor + 1);
+			std::string key( src_key, lenkey );
+			cursor += lenkey + 1;
 
-			index = RFIFOL(fd, cursor);
+			unsigned int  index = RFIFOL(fd, cursor);
 			cursor += 4;
 
 			switch (RFIFOB(fd, cursor++)) {
 				// int
 				case 0:
-					inter_savereg(account_id,char_id,key,index,RFIFOL(fd, cursor),false);
+				{
+					intptr_t lVal = RFIFOL( fd, cursor );
+					inter_savereg( account_id, char_id, key.c_str(), index, lVal, false );
 					cursor += 4;
 					break;
+				}
 				case 1:
-					inter_savereg(account_id,char_id,key,index,0,false);
+					inter_savereg(account_id,char_id,key.c_str(),index,0,false);
 					break;
 				// str
 				case 2:
-					safestrncpy(sval, RFIFOCP(fd, cursor + 1), RFIFOB(fd, cursor));
-					cursor += RFIFOB(fd, cursor) + 1;
-					inter_savereg(account_id,char_id,key,index,(intptr_t)sval,true);
+				{
+					size_t len_val = RFIFOB( fd, cursor );
+					const char* src_val= RFIFOCP(fd, cursor + 1);
+					std::string sval( src_val, len_val );
+					cursor += len_val + 1;
+					inter_savereg( account_id, char_id, key.c_str(), index, (intptr_t)sval.c_str(), true );
 					break;
+				}
 				case 3:
-					inter_savereg(account_id,char_id,key,index,0,true);
+					inter_savereg(account_id,char_id,key.c_str(),index,0,true);
 					break;
 				default:
 					ShowError("mapif_parse_Registry: unknown type %d\n",RFIFOB(fd, cursor - 1));

src/map/intif.cpp

@@ -405,6 +405,7 @@ int intif_saveregistry(struct map_session_data *sd)
 	for( data = iter->first(iter,&key); iter->exists(iter); data = iter->next(iter,&key) ) {
 		const char *varname = NULL;
 		struct script_reg_state *src = NULL;
+		bool lValid = false;
 
 		if( data->type != DB_DATA_PTR ) // it's a @number
 			continue;
@@ -420,13 +421,17 @@ int intif_saveregistry(struct map_session_data *sd)
 			continue;
 
 		src->update = false;
+		lValid = script_check_RegistryVariableLength(0,varname,&len);
+		++len;
 
-		len = strlen(varname)+1;
-
+		if (!lValid) { //this is sql colum size, must be retrive from config
+			ShowError("intif_saveregistry: Variable name length is too long (aid: %d, cid: %d): '%s' sz=%d\n", sd->status.account_id, sd->status.char_id, varname, len);
+			continue;
+		}
 		WFIFOB(inter_fd, plen) = (unsigned char)len; // won't be higher; the column size is 32
 		plen += 1;
 
-		safestrncpy(WFIFOCP(inter_fd,plen), varname, len);
+		safestrncpy(WFIFOCP(inter_fd,plen), varname, len); //the key
 		plen += len;
 
 		WFIFOL(inter_fd, plen) = script_getvaridx(key.i64);
@@ -435,13 +440,19 @@ int intif_saveregistry(struct map_session_data *sd)
 		if( src->type ) {
 			struct script_reg_str *p = (struct script_reg_str *)src;
 
-			WFIFOB(inter_fd, plen) = p->value ? 2 : 3;
+			WFIFOB(inter_fd, plen) = p->value ? 2 : 3; //var type
 			plen += 1;
 
 			if( p->value ) {
-				len = strlen(p->value)+1;
+				lValid = script_check_RegistryVariableLength(1,p->value,&len);
+				++len;
+				if ( !lValid ) { // error can't be higher; the column size is 254. (nb the transmission limit with be fixed with protobuf revamp)
+					ShowDebug( "intif_saveregistry: Variable value length is too long (aid: %d, cid: %d): '%s' sz=%d to be saved with current system and will be truncated\n",sd->status.account_id, sd->status.char_id,p->value,len);
+					len = 254;
+					p->value[len - 1] = '\0'; //this is backward for old char-serv but new one doesn't need this
+				}
 
-				WFIFOB(inter_fd, plen) = (unsigned char)len; // won't be higher; the column size is 254
+				WFIFOB(inter_fd, plen) = len; 
 				plen += 1;
 
 				safestrncpy(WFIFOCP(inter_fd,plen), p->value, len);
@@ -3666,7 +3677,7 @@ int intif_parse_clan_onlinecount( int fd ){
  * @return
  *  0 (unknow packet).
  *  1 sucess (no error)
- *  2 invalid lenght of packet (not enough data yet)
+ *  2 invalid length of packet (not enough data yet)
  */
 int intif_parse(int fd)
 {

src/map/pc.cpp

@@ -9246,12 +9246,19 @@ int pc_setregistry_str(struct map_session_data *sd, int64 reg, const char *val)
 	struct script_reg_str *p = NULL;
 	const char *regname = get_str(script_getvarid(reg));
 	unsigned int index = script_getvaridx(reg);
+	size_t vlen = 0;
 
 	if (!reg_load && !sd->vars_ok) {
 		ShowError("pc_setregistry_str : refusing to set %s until vars are received.\n", regname);
 		return 0;
 	}
 
+	if ( !script_check_RegistryVariableLength(1, val, &vlen ) )
+	{
+		ShowError("pc_check_RegistryVariableLength: Variable value length is too long (aid: %d, cid: %d): '%s' sz=%zu\n", sd->status.account_id, sd->status.char_id, val, vlen);
+		return 0;
+	}
+
 	if( (p = (struct script_reg_str *)i64db_get(sd->regs.vars, reg) ) ) {
 		if( val[0] ) {
 			if( p->value )

src/map/pc.hpp

@@ -1,6 +1,6 @@
 // Copyright (c) Athena Dev Teams - Licensed under GNU GPL
 // For more information, see LICENCE in the main folder
-
+#pragma once
 #ifndef _PC_HPP_
 #define _PC_HPP_
 

src/map/script.cpp

@@ -748,6 +748,21 @@ static unsigned int calc_hash(const char* p)
 	return h % SCRIPT_HASH_SIZE;
 }
 
+bool script_check_RegistryVariableLength(int pType, const char *val, size_t* vlen) 
+{
+	size_t len = strlen(val);
+
+	if (vlen)
+		*vlen = len;
+	switch (pType) {
+		case 0:
+			return (len < 33); // key check
+		case 1:
+			return (len < 255); // value check
+		default:
+			return false;
+	}
+}
 
 /*==========================================
  * str_data manipulation functions
@@ -3124,6 +3139,12 @@ void script_array_update(struct reg_db *src, int64 num, bool empty)
 int set_reg(struct script_state* st, struct map_session_data* sd, int64 num, const char* name, const void* value, struct reg_db *ref)
 {
 	char prefix = name[0];
+	size_t vlen = 0;
+	if ( !script_check_RegistryVariableLength(0,name,&vlen) )
+	{
+		ShowError("set_reg: Variable name length is too long (aid: %d, cid: %d): '%s' sz=%d\n", sd?sd->status.account_id:-1, sd?sd->status.char_id:-1, name, vlen);
+		return 0;
+	}
 
 	if( is_string_variable(name) ) {// string variable
 		const char *str = (const char*)value;

src/map/script.hpp

@@ -1947,4 +1947,6 @@ unsigned int *script_array_cpy_list(struct script_array *sa);
 void queryThread_log(char * entry, int length);
 #endif
 
+bool script_check_RegistryVariableLength(int pType, const char *val, size_t* vlen);
+
 #endif /* _SCRIPT_HPP_ */