Issue Information
-
#000276
-
3 - Medium
-
Fixed
Issue Confirmations
-
Yes (0)No (0)
0
Originally posted by theultramage
http://www.eathena.w...ker&showbug=276
The charserver and mapserver intif code that handles registry loading/saving is messed up.
- out of bounds crash, when the variable is at the beginning of the list (because 'len' will remain uninitialized)
- moving the cursor to a random place in the input buffer (when re-using the previous 'len' value)
- shifting the whole buffer by 1 string, causing names to become values and vice-versa.
As to the conditions that caused the server to save an empty string to the db (seems to be the trigger), I'm not sure about those.
But this has definitely occured roughly 1/2 year ago, and multiple servers have junk in their db since that happened.
This also explains that topic (which I can't find) where a server owner reported a totally messed up globalreg table.
Here are links to revisions where the effects of wild coding resulted in what we have now:
r4480 - changed global_reg value from int to char
r4482 - Realtered global_reg value size to 256, Altered map, char and login server to correctly use global_reg
r4823 (!) - Removed the reg values from mmo_charstatus, these most now be retrieved/obtained individually through packets (char->map load, map->char save); Packet 0x3004 and related to send/receive reg values now use dynamic sizes to be as compact as possible.
http://www.eathena.w...ker&showbug=276
The charserver and mapserver intif code that handles registry loading/saving is messed up.
CODE
for(j=0,p=13;j<ACCOUNT_REG_NUM && p<RFIFOW(fd,2);j++){
sscanf(RFIFOP(fd,p), "%31c%n",reg->reg[j].str,&len);
reg->reg[j].str[len]='\ 0';
p +=len+1; //+1 to skip the '\' between strings.
sscanf(RFIFOP(fd,p), "%255c%n",reg->reg[j].value,&len);
reg->reg[j].value[len]='\ 0';
p +=len+1;
}
This chunk of code (also copy-pasted) is supposed to parse pairs of \ 0-delimited strings. What happens is that if one of them is empty, sscanf will not match it and won't update the 'len' variable. This has multiple outcomessscanf(RFIFOP(fd,p), "%31c%n",reg->reg[j].str,&len);
reg->reg[j].str[len]='\ 0';
p +=len+1; //+1 to skip the '\' between strings.
sscanf(RFIFOP(fd,p), "%255c%n",reg->reg[j].value,&len);
reg->reg[j].value[len]='\ 0';
p +=len+1;
}
- out of bounds crash, when the variable is at the beginning of the list (because 'len' will remain uninitialized)
- moving the cursor to a random place in the input buffer (when re-using the previous 'len' value)
- shifting the whole buffer by 1 string, causing names to become values and vice-versa.
As to the conditions that caused the server to save an empty string to the db (seems to be the trigger), I'm not sure about those.
But this has definitely occured roughly 1/2 year ago, and multiple servers have junk in their db since that happened.
This also explains that topic (which I can't find) where a server owner reported a totally messed up globalreg table.
Here are links to revisions where the effects of wild coding resulted in what we have now:
r4480 - changed global_reg value from int to char
r4482 - Realtered global_reg value size to 256, Altered map, char and login server to correctly use global_reg
r4823 (!) - Removed the reg values from mmo_charstatus, these most now be retrieved/obtained individually through packets (char->map load, map->char save); Packet 0x3004 and related to send/receive reg values now use dynamic sizes to be as compact as possible.
