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

This topic is 4717 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
{
/* Header Information */
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]

##### Share on other sites
Have you checked to see if fopen is actually succeding in opening the file?

##### Share on other sites
Quote:
 Original post by Nemesis2k2Have you checked to see if fopen is actually succeding in opening the file?

Nice call.

I disected the 'f' and found it was broken up into several components. I did some comparisons with other programs I had written to determine if a specific value changed when it loaded sucessfully/differently. It wasn't apparanent.

Than I replace chrUserFile with "1.reg" and ran the program again, and it loaded the data correctly.

I'm not sure why that fixed it, but at least I have a basis to work with now.

Thanks!
rating++;

Edit: Found it! strcpy(*,*), two pointers, so saying strcpy(array, "1.reg") is invalid! I used sprintf(array,"%d.reg",1) and it fixed it.

##### Share on other sites
Well, I suggest that you don't use plain password as you do now, and that you change your name/password strings to std::string, since these are less prone to errors.

So, for your password, I suggest you use MD5 hashes. I used the RSA Data Center C files for this, which you can get here:

Add those files to your project, and then add this function somewhere in your "general purpose functions" files:
string MD5(const string& PlainText){    MD5_CTX       Context;    unsigned char Digest[16];    char          HashValue[33];    HashValue[32] = 0;    MD5Init(&Context);    MD5Update(&Context, (unsigned char *)PlainText.c_str(), (unsigned int)PlainText.length());    MD5Final((unsigned char *) Digest, &Context);    // Copy the digest into the string    int Pos = 0;    for (int i = 0; i < 16; ++i)    {        sprintf(&HashValue[Pos], "%02x", Digest[i]);        Pos += 2;    }    return (HashValue);}

If you don't want to use std::string, it's easy to convert to char*.

Toolmaker

##### Share on other sites

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

## Create an account or sign in to comment

You need to be a member in order to leave a comment

## Create an account

Sign up for a new account in our community. It's easy!

Register a new account

• ### Forum Statistics

• Total Topics
628728
• Total Posts
2984418

• 25
• 11
• 10
• 16
• 14