• 12
• 12
• 9
• 10
• 13

# Only a buffer-overflow is this evil... (Solved)

This topic is 4812 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

I've isolated the error in my MMORPG to the account initialization code. The server isn't loading the account archives from my harddrive correctly: the username & password appear as funny characters when they are suppose to be a series of consecutive characters. Debug Output This is the structure of the data file.
struct strHook
{
char bytUsed;
char bytInGame;
unsigned short intObject;

/* Character Information */
char chrName[15];
char bytType;
unsigned short intTileX;
unsigned short intTileY;
unsigned short intHealth;
unsigned short intMana;
unsigned int intLevel;
unsigned int intSword;
unsigned int intShield;
unsigned int intMagic;
char chrInventory[43];
};


The specific file my server is loading is called "1.reg", and the file contents are shown below. (Note, the account fields are max'd out at 10, but I made them 15 so that I could use the standard string functions. Hence the 5 blank spaces after both the username and password.) Username: aaaaabbbbb Password: cccccddddd The code that loads the file data onto the hook structure
/* This function loads all of the accounts into hooks. */
void sAccInit()
{
FILE *f;
char chrName[15];
unsigned short l;
unsigned short x = 1;
memset(arrHooks,0x0,sizeof(struct strHook) * 10000);
strcpy(chrUserFile,"1.reg");

/* For as long as accounts exist, load them. */
while(GetFileAttributes(chrUserFile) != 0xFFFFFFFF)
{
for(l=0;l<10000;l++)
{
if(arrHooks[l].bytUsed == 0)
{
f=fopen(chrUserFile,"r");

fclose(f);
arrHooks[l].bytUsed = 1;

/* Output the name onto the console. */
memset(chrName,0x0,15);
memcpy(chrName,arrHooks[l].chrName,10);
printf("->"); printf(chrName); printf("\n");
l = 20000;
}
}
/* Set the loop to load the next account. */
x++;
sprintf(chrUserFile, "%d.reg", x);
}
printf("\n");
}


My guess is that somewhere there is a buffer overflow, since the code doesn't look wrong (least not to me). Any suggestions, or ideas on how I can further isolate the problem? [Edited by - ProcedureX on January 16, 2005 4:37:33 PM]

##### Share on other sites
It occurs to me that you are not null-terminating your strings after loading them. I would use binary read/write to files (as opposed to text mode, which has problems with \0) using "rb" instead of "r", and add a '\0' to every string after loading, just to be sure.

##### Share on other sites
Arn't I?

"\0" <=> 0
And since those char[15] have 10 occupied values, and 5 values that equal 0, they both have 5 null terminators on the end. [razz]

fread "r" has problems with "\0"? Interesting, I'll google it, and see if those problems apply to my code. [smile]

##### Share on other sites
1. You are hoping that the value read from a file is "correct", which you shouldn't (never trust anything you don't have absolute control over, and a file does not qualify for this).

2. If I remember correctly, a value of 0 in text mode is read incorrectly as EOF (I might be wrong, however), which is why I suggested reading in binary mode.

##### Share on other sites
Quote:
 Original post by ToohrVyk2. If I remember correctly, a value of 0 in text mode is read incorrectly as EOF (I might be wrong, however), which is why I suggested reading in binary mode.

IC.

Well, I changed my server to read "rb", inaddition, I modified my CGI code to write to "wb" (Since the accounts are generated via a web interface). I recompiled em, and ran em. The output was identical. [bawling]

##### Share on other sites
Adding b only has an effect in ANSI C. I assume you are using ISO C.

##### Share on other sites
I am using which ever standard of C is implmented in Visual Studio 6.

Edit: It is indeed a tough problem... [sad]

[Edited by - ProcedureX on January 16, 2005 2:16:47 PM]

##### Share on other sites
Two things. First of all, you're using fread incorrectly. Your reads should be structured more like this:

//Use the third parameter to specify number of elementsfread(&arrHooks[l].chrName[0],sizeof(char),15,f);//NOT like this:fread(&arrHooks[l].chrName[0],sizeof(char)*15,1,f);

Secondly, you're only trying to read in 43 characters for the inventory, not 44. You need to set the read count to 44 for the inventory data.

##### Share on other sites
Quote:
 Original post by Nemesis2k2Two things. First of all, you're using fread incorrectly. Your reads should be structured more like this:*** Source Snippet Removed ***

Yup, I read that when I googled the thingy. However, I knew it wouldn't make a different because I've never used it like that before. Nevertheless, because you mentioned it, I changed it. It didn't seem to make any difference though.

Quote:
 Original post by Nemesis2k2Secondly, you're only trying to read in 43 characters for the inventory, not 44. You need to set the read count to 44 for the inventory data.

lol, am I? Didn't notice. Doesn't fix this problem though, but I'm sure you knew that. ;)

Edit: Wow, lol, this is the first time I've posted a thread that wasn't solved within minutes.

[Edited by - ProcedureX on January 16, 2005 3:52:13 PM]