Merge pull request #2705 from Kenpachi2k13/string_var_size

Cap string variable value length and unify corresponding SQL columns size

12 files changed, 85 insertions, 35 deletions

diff --git a/src/char/char.c b/src/char/char.c
index 0406dbecf..c61b6107a 100644
--- a/src/char/char.c
+++ b/src/char/char.c
@@ -2902,13 +2902,13 @@ static void char_global_accreg_to_login_add(const char *key, unsigned int index,
 
 		if( val ) {
 			char *sval = (char*)val;
-			len = strlen(sval)+1;
+			len = strlen(sval);
 
-			WFIFOB(chr->login_fd, nlen) = (unsigned char)len;/* won't be higher; the column size is 254 */
+			WFIFOB(chr->login_fd, nlen) = (unsigned char)len; // Won't be higher; the column size is 255.
 			nlen += 1;
 
-			safestrncpy(WFIFOP(chr->login_fd,nlen), sval, len);
-			nlen += len;
+			safestrncpy(WFIFOP(chr->login_fd, nlen), sval, len + 1);
+			nlen += len + 1;
 		}
 	} else {
 		WFIFOB(chr->login_fd, nlen) = val ? 0 : 1;
diff --git a/src/char/inter.c b/src/char/inter.c
index 2d8d06a9c..5252b3315 100644
--- a/src/char/inter.c
+++ b/src/char/inter.c
@@ -676,13 +676,13 @@ static int inter_accreg_fromsql(int account_id, int char_id, int fd, int type)
 		plen += 4;
 
 		SQL->GetData(inter->sql_handle, 2, &data, NULL);
-		len = strlen(data)+1;
+		len = strlen(data);
 
-		WFIFOB(fd, plen) = (unsigned char)len;/* won't be higher; the column size is 254 */
+		WFIFOB(fd, plen) = (unsigned char)len; // Won't be higher; the column size is 255.
 		plen += 1;
 
-		safestrncpy(WFIFOP(fd,plen), data, len);
-		plen += len;
+		safestrncpy(WFIFOP(fd, plen), data, len + 1);
+		plen += len + 1;
 
 		WFIFOW(fd, 14) += 1;
 
diff --git a/src/char/mapif.c b/src/char/mapif.c
index 9077afae4..f0c886586 100644
--- a/src/char/mapif.c
+++ b/src/char/mapif.c
@@ -2030,7 +2030,8 @@ static int mapif_parse_Registry(int fd)
 
 	if (count != 0) {
 		int cursor = 14, i;
-		char key[SCRIPT_VARNAME_LENGTH+1], sval[254];
+		char key[SCRIPT_VARNAME_LENGTH + 1];
+		char sval[SCRIPT_STRING_VAR_LENGTH + 1];
 		bool isLoginActive = sockt->session_is_active(chr->login_fd);
 
 		if (isLoginActive)
@@ -2057,8 +2058,8 @@ static int mapif_parse_Registry(int fd)
 			/* str */
 			case 2:
 				len = RFIFOB(fd, cursor);
-				safestrncpy(sval, RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len));
-				cursor += len + 1;
+				safestrncpy(sval, RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len + 1));
+				cursor += len + 2;
 				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 9421f6e35..d2f3aa8f1 100644
--- a/src/common/mmo.h
+++ b/src/common/mmo.h
@@ -380,7 +380,10 @@ STATIC_ASSERT(MAX_ITEM_OPTIONS <= 5, "This value is limited by the client and da
 #define JOBL_BABY  0x2000
 #define JOBL_THIRD 0x4000
 
-#define SCRIPT_VARNAME_LENGTH 32 ///< Maximum length of a script variable
+#define SCRIPT_VARNAME_LENGTH 32 ///< Maximum length of a script variable's name including affixes and excluding NULL-terminator.
+STATIC_ASSERT(SCRIPT_VARNAME_LENGTH <= 32, "This value is limited by the inter-server communication and database layout and should only be increased if you know the consequences.");
+#define SCRIPT_STRING_VAR_LENGTH 255 ///< Maximum length of strings stored in script variables excluding NULL-terminator.
+STATIC_ASSERT(SCRIPT_STRING_VAR_LENGTH <= 255, "This value is limited by the inter-server communication and database layout and should only be increased if you know the consequences.");
 
 #define INFINITE_DURATION (-1) // Infinite duration for status changes
 
diff --git a/src/login/account.c b/src/login/account.c
index 3632c257a..ec0bc81e8 100644
--- a/src/login/account.c
+++ b/src/login/account.c
@@ -632,7 +632,8 @@ static void account_mmo_save_accreg2(AccountDB *self, int fd, int account_id, in
 	sql_handle = db->accounts;
 	if (count) {
 		int cursor = 14, i;
-		char key[SCRIPT_VARNAME_LENGTH+1], sval[254];
+		char key[SCRIPT_VARNAME_LENGTH + 1];
+		char sval[SCRIPT_STRING_VAR_LENGTH + 1];
 
 		for (i = 0; i < count; i++) {
 			unsigned int index;
@@ -657,8 +658,8 @@ static void account_mmo_save_accreg2(AccountDB *self, int fd, int account_id, in
 				/* str */
 				case 2:
 					len = RFIFOB(fd, cursor);
-					safestrncpy(sval, RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len));
-					cursor += len + 1;
+					safestrncpy(sval, RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len + 1));
+					cursor += len + 2;
 					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;
@@ -719,13 +720,13 @@ static void account_mmo_send_accreg2(AccountDB *self, int fd, int account_id, in
 		plen += 4;
 
 		SQL->GetData(sql_handle, 2, &data, NULL);
-		len = strlen(data)+1;
+		len = strlen(data);
 
-		WFIFOB(fd, plen) = (unsigned char)len;/* won't be higher; the column size is 254 */
+		WFIFOB(fd, plen) = (unsigned char)len; // Won't be higher; the column size is 255.
 		plen += 1;
 
-		safestrncpy(WFIFOP(fd,plen), data, len);
-		plen += len;
+		safestrncpy(WFIFOP(fd, plen), data, len + 1);
+		plen += len + 1;
 
 		WFIFOW(fd, 14) += 1;
 
diff --git a/src/map/atcommand.c b/src/map/atcommand.c
index 3fe6c8581..54cc9e2c9 100644
--- a/src/map/atcommand.c
+++ b/src/map/atcommand.c
@@ -8916,13 +8916,17 @@ ACMD(accinfo)
 /* [Ind] */
 ACMD(set)
 {
-	char reg[SCRIPT_VARNAME_LENGTH+1], val[254];
+	char reg[SCRIPT_VARNAME_LENGTH + 1];
+	char val[SCRIPT_STRING_VAR_LENGTH + 1];
 	struct script_data* data;
 	int toset = 0;
 	bool is_str = false;
 	size_t len;
 
-	if (!*message || (toset = sscanf(message, "%32s %253[^\n]", reg, val)) < 1) {
+	char format[20];
+	safesnprintf(format, sizeof(format), "%%%ds %%%d[^\\n]", SCRIPT_VARNAME_LENGTH, SCRIPT_STRING_VAR_LENGTH);
+
+	if (*message == '\0' || (toset = sscanf(message, format, 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/clif.c b/src/map/clif.c
index 7be5c6978..ab13ffe1f 100644
--- a/src/map/clif.c
+++ b/src/map/clif.c
@@ -11794,7 +11794,7 @@ static void clif_parse_WisMessage(int fd, struct map_session_data *sd)
 		char *str = target + 4; // Skip the NPC: string part.
 		struct npc_data *nd;
 		if ((nd = npc->name2id(str))) {
-			char split_data[NUM_WHISPER_VAR][CHAT_SIZE_MAX];
+			char split_data[NUM_WHISPER_VAR][SCRIPT_STRING_VAR_LENGTH + 1];
 			char *split;
 			char output[256];
 
diff --git a/src/map/intif.c b/src/map/intif.c
index a420b7204..276b40780 100644
--- a/src/map/intif.c
+++ b/src/map/intif.c
@@ -218,13 +218,13 @@ static int intif_saveregistry(struct map_session_data *sd)
 			plen += 1;
 
 			if( p->value ) {
-				len = strlen(p->value)+1;
+				len = strlen(p->value);
 
-				WFIFOB(inter_fd, plen) = (unsigned char)len;/* won't be higher; the column size is 254 */
+				WFIFOB(inter_fd, plen) = (unsigned char)len; // Won't be higher; the column size is 255.
 				plen += 1;
 
-				safestrncpy(WFIFOP(inter_fd,plen), p->value, len);
-				plen += len;
+				safestrncpy(WFIFOP(inter_fd, plen), p->value, len + 1);
+				plen += len + 1;
 			} else {
 				script->reg_destroy_single(sd,key.i64,&p->flag);
 			}
@@ -1025,7 +1025,7 @@ static void intif_parse_Registers(int fd)
 		 * { keyLength(B), key(<keyLength>), index(L), valLength(B), val(<valLength>) }
 		 **/
 		if (type) {
-			char sval[254];
+			char sval[SCRIPT_STRING_VAR_LENGTH + 1];
 			for (i = 0; i < max; i++) {
 				int len = RFIFOB(fd, cursor);
 				safestrncpy(key, RFIFOP(fd, cursor + 1), min((int)sizeof(key), len));
@@ -1035,8 +1035,8 @@ static void intif_parse_Registers(int fd)
 				cursor += 4;
 
 				len = RFIFOB(fd, cursor);
-				safestrncpy(sval, RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len));
-				cursor += len + 1;
+				safestrncpy(sval, RFIFOP(fd, cursor + 1), min((int)sizeof(sval), len + 1));
+				cursor += len + 2;
 
 				script->set_reg(NULL,sd,reference_uid(script->add_variable(key), index), key, sval, NULL);
 			}
diff --git a/src/map/mapreg_sql.c b/src/map/mapreg_sql.c
index 741505e17..2963637da 100644
--- a/src/map/mapreg_sql.c
+++ b/src/map/mapreg_sql.c
@@ -176,9 +176,9 @@ static bool mapreg_setregstr(int64 uid, const char *str)
 
 			if(name[1] != '@' && !mapreg->skip_insert) { //put returned null, so we must insert.
 				char tmp_str[(SCRIPT_VARNAME_LENGTH+1)*2+1];
-				char tmp_str2[255*2+1];
+				char tmp_str2[SCRIPT_STRING_VAR_LENGTH * 2 + 1];
 				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));
+				SQL->EscapeStringLen(map->mysql_handle, tmp_str2, str, strnlen(str, SCRIPT_STRING_VAR_LENGTH));
 				if( SQL_ERROR == SQL->Query(map->mysql_handle, "INSERT INTO `%s`(`varname`,`index`,`value`) VALUES ('%s','%u','%s')", mapreg->table, tmp_str, i, tmp_str2) )
 					Sql_ShowDebug(map->mysql_handle);
 			}
@@ -203,7 +203,7 @@ static void script_load_mapreg(void)
 	struct SqlStmt *stmt = SQL->StmtMalloc(map->mysql_handle);
 	char varname[SCRIPT_VARNAME_LENGTH+1];
 	int index;
-	char value[255+1];
+	char value[SCRIPT_STRING_VAR_LENGTH + 1];
 	uint32 length;
 
 	if ( SQL_ERROR == SQL->StmtPrepare(stmt, "SELECT `varname`, `index`, `value` FROM `%s`", mapreg->table)
@@ -261,8 +261,8 @@ static void script_save_mapreg(void)
 					if( SQL_ERROR == SQL->Query(map->mysql_handle, "UPDATE `%s` SET `value`='%d' WHERE `varname`='%s' AND `index`='%d' LIMIT 1", mapreg->table, m->u.i, name, i) )
 						Sql_ShowDebug(map->mysql_handle);
 				} else {
-					char tmp_str2[2*255+1];
-					SQL->EscapeStringLen(map->mysql_handle, tmp_str2, m->u.str, safestrnlen(m->u.str, 255));
+					char tmp_str2[SCRIPT_STRING_VAR_LENGTH * 2 + 1];
+					SQL->EscapeStringLen(map->mysql_handle, tmp_str2, m->u.str, safestrnlen(m->u.str, SCRIPT_STRING_VAR_LENGTH));
 					if( SQL_ERROR == SQL->Query(map->mysql_handle, "UPDATE `%s` SET `value`='%s' WHERE `varname`='%s' AND `index`='%d' LIMIT 1", mapreg->table, tmp_str2, name, i) )
 						Sql_ShowDebug(map->mysql_handle);
 				}
diff --git a/src/map/npc_chat.c b/src/map/npc_chat.c
index 0ca84cff4..0df323e96 100644
--- a/src/map/npc_chat.c
+++ b/src/map/npc_chat.c
@@ -394,7 +394,8 @@ static int npc_chat_sub(struct block_list *bl, va_list ap)
 				// save out the matched strings
 				for (i = 0; i < r; i++)
 				{
-					char var[15], val[255];
+					char var[SCRIPT_VARNAME_LENGTH + 1];
+					char val[SCRIPT_STRING_VAR_LENGTH + 1];
 					snprintf(var, sizeof(var), "$@p%i$", i);
 					libpcre->copy_substring(msg, offsets, r, i, val, sizeof(val));
 					script->set_var(sd, var, val);
diff --git a/src/map/script.c b/src/map/script.c
index 45c954dc0..347ef3d2a 100644
--- a/src/map/script.c
+++ b/src/map/script.c
@@ -3395,6 +3395,32 @@ static void set_reg_instance_num(struct script_state *st, int64 num, const char
 }
 
 /**
+ * Validates if a variable is permanent (stored in database) by passed variable name.
+ *
+ * @param name The variable name to validate.
+ * @return True if variable is permanent, otherwise false.
+ *
+ **/
+static bool script_is_permanent_variable(const char *name)
+{
+	nullpo_retr(false, name);
+
+	if (strlen(name) == 0)
+		return false;
+
+	if (ISALNUM(name[0]) != 0)
+		return true; // Permanent characater variable.
+
+	if (name[0] == '#')
+		return true; // Permanent (global) account variable.
+
+	if (strlen(name) > 1 && name[0] == '$' && ISALNUM(name[1]) != 0)
+		return true; // Permanent server variable.
+
+	return false;
+}
+
+/**
  * Stores the value of a script variable
  *
  * @param st    current script state.
@@ -3439,6 +3465,18 @@ static int set_reg(struct script_state *st, struct map_session_data *sd, int64 n
 	if (is_string_variable(name)) {// string variable
 		const char *str = (const char*)value;
 
+		if (script->is_permanent_variable(name) && strlen(str) > SCRIPT_STRING_VAR_LENGTH) {
+			ShowError("script:set_reg: Value of variable %s is too long: %lu! Maximum is %d. Skipping...\n",
+				  name, strlen(str), SCRIPT_STRING_VAR_LENGTH);
+
+			if (st != NULL) {
+				script->reportsrc(st);
+				st->state = END;
+			}
+
+			return 0;
+		}
+
 		switch (prefix) {
 		case '@':
 			if (ref) {
@@ -28255,6 +28293,7 @@ void script_defaults(void)
 	script->load_parameters = script_load_parameters;
 	script->print_line = script_print_line;
 	script->errorwarning_sub = script_errorwarning_sub;
+	script->is_permanent_variable = script_is_permanent_variable;
 	script->set_reg = set_reg;
 	script->set_reg_ref_str = set_reg_npcscope_str;
 	script->set_reg_pc_ref_str = set_reg_pc_ref_str;
diff --git a/src/map/script.h b/src/map/script.h
index 511497a66..5fa81dc0e 100644
--- a/src/map/script.h
+++ b/src/map/script.h
@@ -984,6 +984,7 @@ struct script_interface {
 	void (*load_parameters) (void);
 	const char* (*print_line) (StringBuf *buf, const char *p, const char *mark, int line);
 	void (*errorwarning_sub) (StringBuf *buf, const char *src, const char *file, int start_line, const char *error_msg, const char *error_pos);
+	bool (*is_permanent_variable) (const char *name);
 	int (*set_reg) (struct script_state *st, struct map_session_data *sd, int64 num, const char *name, const void *value, struct reg_db *ref);
 	void (*set_reg_ref_str) (struct script_state* st, struct reg_db *n, int64 num, const char* name, const char *str);
 	void (*set_reg_pc_ref_str) (struct script_state* st, struct reg_db *n, int64 num, const char* name, const char *str);