Merge pull request #1003 from HerculesWS/865-var_length

Merge of #866 and #867

8 files changed, 61 insertions, 31 deletions

diff --git a/doc/script_commands.txt b/doc/script_commands.txt
index ef4816889..c08596f9a 100644
--- a/doc/script_commands.txt
+++ b/doc/script_commands.txt
@@ -525,7 +525,8 @@ If a variable was never set, it is considered to equal zero for integer
 variables or an empty string ("", nothing between the quotes) for string 
 variables. Once you set it to that, the variable is as good as forgotten 
 forever, and no trace remains of it even if it was stored with character 
-or account data.
+or account data. The maximum length of variable name including prefix and
+suffix is 32.
 
 Some variables are special, that is, they are already defined for you by 
 the scripting engine. You can see the full list somewhere in 
diff --git a/src/char/inter.c b/src/char/inter.c
index 5b81a4732..87ecb4e6a 100644
--- a/src/char/inter.c
+++ b/src/char/inter.c
@@ -1186,7 +1186,7 @@ int mapif_parse_Registry(int fd)
 
 	if( count ) {
 		int cursor = 14, i;
-		char key[32], sval[254];
+		char key[SCRIPT_VARNAME_LENGTH+1], sval[254];
 		bool isLoginActive = sockt->session_is_active(chr->login_fd);
 
 		if( isLoginActive )
@@ -1194,8 +1194,9 @@ int mapif_parse_Registry(int fd)
 
 		for(i = 0; i < count; i++) {
 			unsigned int index;
-			safestrncpy(key, (char*)RFIFOP(fd, cursor + 1), RFIFOB(fd, cursor));
-			cursor += RFIFOB(fd, cursor) + 1;
+			int len = RFIFOB(fd, cursor);
+			safestrncpy(key, (char*)RFIFOP(fd, cursor + 1), min((int)sizeof(key), len));
+			cursor += len + 1;
 
 			index = RFIFOL(fd, cursor);
 			cursor += 4;
@@ -1211,8 +1212,9 @@ int mapif_parse_Registry(int fd)
 					break;
 				/* str */
 				case 2:
-					safestrncpy(sval, (char*)RFIFOP(fd, cursor + 1), RFIFOB(fd, cursor));
-					cursor += RFIFOB(fd, cursor) + 1;
+					len = RFIFOB(fd, cursor);
+					safestrncpy(sval, (char*)RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len));
+					cursor += len + 1;
 					inter->savereg(account_id,char_id,key,index,(intptr_t)sval,true);
 					break;
 				case 3:
diff --git a/src/common/mmo.h b/src/common/mmo.h
index cb8a75b24..8444a8d67 100644
--- a/src/common/mmo.h
+++ b/src/common/mmo.h
@@ -214,6 +214,8 @@
 #define JOBL_BABY 0x2000  //8192
 #define JOBL_THIRD 0x4000 //16384
 
+#define SCRIPT_VARNAME_LENGTH 32 ///< Maximum length of a script variable
+
 struct hplugin_data_store;
 
 enum item_types {
diff --git a/src/login/account_sql.c b/src/login/account_sql.c
index 89f4aaaab..1de0fb5e9 100644
--- a/src/login/account_sql.c
+++ b/src/login/account_sql.c
@@ -714,12 +714,13 @@ void mmo_save_accreg2(AccountDB* self, int fd, int account_id, int char_id) {
 	sql_handle = db->accounts;
 	if (count) {
 		int cursor = 14, i;
-		char key[32], sval[254];
+		char key[SCRIPT_VARNAME_LENGTH+1], sval[254];
 
 		for (i = 0; i < count; i++) {
 			unsigned int index;
-			safestrncpy(key, (char*)RFIFOP(fd, cursor + 1), RFIFOB(fd, cursor));
-			cursor += RFIFOB(fd, cursor) + 1;
+			int len = RFIFOB(fd, cursor);
+			safestrncpy(key, (char*)RFIFOP(fd, cursor + 1), min((int)sizeof(key), len));
+			cursor += len + 1;
 
 			index = RFIFOL(fd, cursor);
 			cursor += 4;
@@ -737,8 +738,9 @@ void mmo_save_accreg2(AccountDB* self, int fd, int account_id, int char_id) {
 					break;
 				/* str */
 				case 2:
-					safestrncpy(sval, (char*)RFIFOP(fd, cursor + 1), RFIFOB(fd, cursor));
-					cursor += RFIFOB(fd, cursor) + 1;
+					len = RFIFOB(fd, cursor);
+					safestrncpy(sval, (char*)RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len));
+					cursor += len + 1;
 					if( SQL_ERROR == SQL->Query(sql_handle, "REPLACE INTO `%s` (`account_id`,`key`,`index`,`value`) VALUES ('%d','%s','%u','%s')", db->global_acc_reg_str_db, account_id, key, index, sval) )
 						Sql_ShowDebug(sql_handle);
 					break;
diff --git a/src/map/atcommand.c b/src/map/atcommand.c
index b284323fd..96a2e0c2f 100644
--- a/src/map/atcommand.c
+++ b/src/map/atcommand.c
@@ -8436,14 +8436,15 @@ ACMD(accinfo) {
 }
 
 /* [Ind] */
-ACMD(set) {
-	char reg[32], val[128];
+ACMD(set)
+{
+	char reg[SCRIPT_VARNAME_LENGTH+1], val[254];
 	struct script_data* data;
 	int toset = 0;
 	bool is_str = false;
 	size_t len;
 
-	if (!*message || (toset = sscanf(message, "%31s %127[^\n]s", reg, val)) < 1) {
+	if (!*message || (toset = sscanf(message, "%32s %253[^\n]", reg, val)) < 1) {
 		clif->message(fd, msg_fd(fd,1367)); // Usage: @set <variable name> <value>
 		clif->message(fd, msg_fd(fd,1368)); // Usage: ex. "@set PoringCharVar 50"
 		clif->message(fd, msg_fd(fd,1369)); // Usage: ex. "@set PoringCharVarSTR$ Super Duper String"
diff --git a/src/map/intif.c b/src/map/intif.c
index 06b910d54..016b4f7d3 100644
--- a/src/map/intif.c
+++ b/src/map/intif.c
@@ -333,6 +333,10 @@ int intif_saveregistry(struct map_session_data *sd) {
 		if( varname[0] == '@' ) /* @string$ can get here, so we skip */
 			continue;
 
+		if (strlen(varname) > SCRIPT_VARNAME_LENGTH) {
+			ShowError("Variable name too big: %s\n", varname);
+			continue;
+		}
 		src = DB->data2ptr(data);
 
 		/* no need! */
@@ -1077,8 +1081,8 @@ void intif_parse_Registers(int fd)
 	/* have it not complain about insertion of vars before loading, and not set those vars as new or modified */
 	pc->reg_load = true;
 
-	if( RFIFOW(fd, 14) ) {
-		char key[32];
+	if (RFIFOW(fd, 14) != 0) {
+		char key[SCRIPT_VARNAME_LENGTH+1];
 		unsigned int index;
 		int max = RFIFOW(fd, 14), cursor = 16, i;
 
@@ -1091,16 +1095,18 @@ void intif_parse_Registers(int fd)
 		 * { keyLength(B), key(<keyLength>), index(L), valLength(B), val(<valLength>) }
 		 **/
 		if (type) {
-			for(i = 0; i < max; i++) {
-				char sval[254];
-				safestrncpy(key, (char*)RFIFOP(fd, cursor + 1), RFIFOB(fd, cursor));
-				cursor += RFIFOB(fd, cursor) + 1;
+			char sval[254];
+			for (i = 0; i < max; i++) {
+				int len = RFIFOB(fd, cursor);
+				safestrncpy(key, (char*)RFIFOP(fd, cursor + 1), min((int)sizeof(key), len));
+				cursor += len + 1;
 
 				index = RFIFOL(fd, cursor);
 				cursor += 4;
 
-				safestrncpy(sval, (char*)RFIFOP(fd, cursor + 1), RFIFOB(fd, cursor));
-				cursor += RFIFOB(fd, cursor) + 1;
+				len = RFIFOB(fd, cursor);
+				safestrncpy(sval, (char*)RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len));
+				cursor += len + 1;
 
 				script->set_reg(NULL,sd,reference_uid(script->add_str(key), index), key, (void*)sval, NULL);
 			}
@@ -1111,10 +1117,12 @@ void intif_parse_Registers(int fd)
 		 * { keyLength(B), key(<keyLength>), index(L), value(L) }
 		 **/
 		} else {
-			for(i = 0; i < max; i++) {
+			for (i = 0; i < max; i++) {
 				int ival;
-				safestrncpy(key, (char*)RFIFOP(fd, cursor + 1), RFIFOB(fd, cursor));
-				cursor += RFIFOB(fd, cursor) + 1;
+
+				int len = RFIFOB(fd, cursor);
+				safestrncpy(key, (char*)RFIFOP(fd, cursor + 1), min((int)sizeof(key), len));
+				cursor += len + 1;
 
 				index = RFIFOL(fd, cursor);
 				cursor += 4;
diff --git a/src/map/mapreg_sql.c b/src/map/mapreg_sql.c
index ddd259651..9bf67196e 100644
--- a/src/map/mapreg_sql.c
+++ b/src/map/mapreg_sql.c
@@ -94,9 +94,9 @@ bool mapreg_setreg(int64 uid, int val) {
 			m->save = false;
 			m->is_string = false;
 
-			if(name[1] != '@' && !mapreg->skip_insert) {// write new variable to database
-				char tmp_str[32*2+1];
-				SQL->EscapeStringLen(map->mysql_handle, tmp_str, name, strnlen(name, 32));
+			if (name[1] != '@' && !mapreg->skip_insert) {// write new variable to database
+				char tmp_str[(SCRIPT_VARNAME_LENGTH+1)*2+1];
+				SQL->EscapeStringLen(map->mysql_handle, tmp_str, name, strnlen(name, SCRIPT_VARNAME_LENGTH+1));
 				if( SQL_ERROR == SQL->Query(map->mysql_handle, "INSERT INTO `%s`(`varname`,`index`,`value`) VALUES ('%s','%d','%d')", mapreg->table, tmp_str, i, val) )
 					Sql_ShowDebug(map->mysql_handle);
 			}
@@ -166,9 +166,9 @@ bool mapreg_setregstr(int64 uid, const char* str) {
 			m->is_string = true;
 
 			if(name[1] != '@' && !mapreg->skip_insert) { //put returned null, so we must insert.
-				char tmp_str[32*2+1];
+				char tmp_str[(SCRIPT_VARNAME_LENGTH+1)*2+1];
 				char tmp_str2[255*2+1];
-				SQL->EscapeStringLen(map->mysql_handle, tmp_str, name, strnlen(name, 32));
+				SQL->EscapeStringLen(map->mysql_handle, tmp_str, name, strnlen(name, SCRIPT_VARNAME_LENGTH+1));
 				SQL->EscapeStringLen(map->mysql_handle, tmp_str2, str, strnlen(str, 255));
 				if( SQL_ERROR == SQL->Query(map->mysql_handle, "INSERT INTO `%s`(`varname`,`index`,`value`) VALUES ('%s','%d','%s')", mapreg->table, tmp_str, i, tmp_str2) )
 					Sql_ShowDebug(map->mysql_handle);
@@ -191,7 +191,7 @@ void script_load_mapreg(void) {
 	   +-------------------------+
 	                                */
 	SqlStmt* stmt = SQL->StmtMalloc(map->mysql_handle);
-	char varname[32+1];
+	char varname[SCRIPT_VARNAME_LENGTH+1];
 	int index;
 	char value[255+1];
 	uint32 length;
diff --git a/src/map/script.c b/src/map/script.c
index 7a5292159..829555820 100644
--- a/src/map/script.c
+++ b/src/map/script.c
@@ -2726,6 +2726,13 @@ struct script_data *get_val(struct script_state* st, struct script_data* data) {
 	prefix = name[0];
 	postfix = name[strlen(name) - 1];
 
+	if (strlen(name) > SCRIPT_VARNAME_LENGTH) {
+		ShowError("script_get_val: variable name too long. '%s'\n", name);
+		script->reportsrc(st);
+		st->state = END;
+		return data;
+	}
+
 	//##TODO use reference_tovariable(data) when it's confirmed that it works [FlavioJS]
 	if( !reference_toconstant(data) && not_server_variable(prefix) ) {
 		sd = script->rid2sd(st);
@@ -3142,6 +3149,13 @@ void set_reg_instance_num(struct script_state* st, int64 num, const char* name,
 int set_reg(struct script_state* st, TBL_PC* sd, int64 num, const char* name, const void* value, struct reg_db *ref) {
 	char prefix = name[0];
 
+	if (strlen(name) > SCRIPT_VARNAME_LENGTH) {
+		ShowError("script:set_reg: variable name too long. '%s'\n", name);
+		script->reportsrc(st);
+		st->state = END;
+		return 0;
+	}
+
 	if( is_string_variable(name) ) {// string variable
 		const char *str = (const char*)value;