From ce6eafb3ec39bf38384a944531b63abf452c80fe Mon Sep 17 00:00:00 2001 From: AnnieRuru Date: Mon, 16 Nov 2015 01:35:24 +0800 Subject: Add a source constant SCRIPT_VARNAME_LENGTH since we can freely adjust the length of the variable name just edit this value and edit in main.sql --- src/common/mmo.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') 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 { -- cgit v1.2.3-70-g09d2 From b5021bf40bb1d0a6d38d7b85789703dc12a26180 Mon Sep 17 00:00:00 2001 From: Haru Date: Tue, 22 Dec 2015 03:29:39 +0100 Subject: Ensured 32+1 bytes for all buffers that hold variable names Related: #865, #866, #867 Signed-off-by: Haru --- src/char/inter.c | 12 +++++++----- src/login/account_sql.c | 12 +++++++----- src/map/atcommand.c | 7 ++++--- src/map/intif.c | 26 +++++++++++++++----------- src/map/mapreg_sql.c | 12 ++++++------ 5 files changed, 39 insertions(+), 30 deletions(-) (limited to 'src') 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/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 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..8066d07b9 100644 --- a/src/map/intif.c +++ b/src/map/intif.c @@ -1077,8 +1077,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 +1091,18 @@ void intif_parse_Registers(int fd) * { keyLength(B), key(), index(L), valLength(B), val() } **/ 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 +1113,12 @@ void intif_parse_Registers(int fd) * { keyLength(B), key(), 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; -- cgit v1.2.3-70-g09d2 From c276e2a6aa566400b1026236e7b2c09bfdcb72b5 Mon Sep 17 00:00:00 2001 From: Andrei Karas Date: Sun, 15 Nov 2015 18:46:04 +0300 Subject: Not allow send too big variable names to char server from map server (Closes #865) Closes #866 as merged --- src/map/intif.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src') diff --git a/src/map/intif.c b/src/map/intif.c index 8066d07b9..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! */ -- cgit v1.2.3-70-g09d2 From feff016d6bc7e811d03c958da754df20f630b33b Mon Sep 17 00:00:00 2001 From: AnnieRuru Date: Mon, 16 Nov 2015 01:04:55 +0800 Subject: Throw error when variable name length too long --- doc/script_commands.txt | 3 ++- src/map/script.c | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) (limited to 'src') 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/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; -- cgit v1.2.3-70-g09d2