Jump to content
  • Advertisement
Sign in to follow this  
ProcedureX

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

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

If you intended to correct an error in the post then please contact us.

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 Image Hosted by ImageShack.us 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 chrPassword[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 Image Hosted by ImageShack.us 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");
				fread(&arrHooks[l].chrName[0],sizeof(char)*15,1,f);
				fread(&arrHooks[l].chrPassword[0],sizeof(char)*15,1,f);
				fread(&arrHooks[l].bytType,sizeof(char),1,f);

				fread(&arrHooks[l].intTileX,sizeof(short),1,f);
				fread(&arrHooks[l].intTileY,sizeof(short),1,f);
				fread(&arrHooks[l].intHealth, sizeof(short),1,f);
				fread(&arrHooks[l].intMana, sizeof(short),1,f);

				fread(&arrHooks[l].intLevel, sizeof(int),1,f);
				fread(&arrHooks[l].intSword, sizeof(int),1,f);
				fread(&arrHooks[l].intShield, sizeof(int),1,f);
				fread(&arrHooks[l].intMagic, sizeof(int),1,f);
				fread(&arrHooks[l].chrInventory[0],sizeof(char) * 43,1,f);
				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 this post


Link to post
Share on other sites
Advertisement
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
Share on other sites
Quote:
Original post by ToohrVyk
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.


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 this post


Link to post
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 this post


Link to post
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 elements
fread(&arrHooks[l].chrName[0],sizeof(char),15,f);

//NOT like this:
fread(&arrHooks[l].chrName[0],sizeof(char)*15,1,f);




Read the description for fread from MSDN for more information.

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 this post


Link to post
Share on other sites
Quote:
Original post by Nemesis2k2
Two 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 Nemesis2k2
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.


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 this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!