C Board  

Go Back   C Board > General Programming Boards > C Programming

Reply
 
LinkBack Thread Tools Display Modes
Old 06-17-2008, 11:51 AM   #1
Registered User
 
Join Date: Jun 2008
Posts: 4
Sprintf overflows my buffer -- why?

First, the code:

Code:
main(int argc, char * const argv[]) {
	char *ident_a=0; // Initialize ident_a as a null pointer so we can easily check for that later
	ident_a=malloc(sizeof(the_argument)); // dynamic memory allocation for ident_a
	ident_a=strcpy(ident_a, the_argument); // should be safe since enough memory was allotted

	testident(ident_a);
}

testident(char *ident_a) {	
	/*
	* Makes a string with a random number in it
	*/
	if(ident_a != 0) { // As long as ident_a isn't a null pointer, test it
		char *rstring=malloc(sizeof(int)+sizeof("dark_ok__stupid")); // Allocates enough memory for the string we're making (string + enough for random number)
		sprintf(rstring, "dark_ok_%d_stupid", rand()); // Fills our array with a string with a random number in it
		// Immediately after this sprintf, in testing, is where ident_a changes and gets filled with some of what sprintf() is writing

		/* rstring is freed */
	}
}
This is obviously very stripped own, but I didn't want to confuse people with too much irrelevant code.

The ident_a argument is received well, I've checked and no matter how large it stays in its place.

The problem is, when I do the sprintf() later in the program, it somehow writes to ident_a. As in, if I put abcdefghijklmnopqrstuvwxy0123456789 it would cut off after the "p" and fill the rest of ident_a with "dark_ok_RANDOMNUMBER_stupid" (obviously with a real random number in place of RANDOMNUMBER).

Why is my sprintf writing to ident_a? Didn't I give it enough memory?

Any help would be appreciated, thanks.
Lasston is offline   Reply With Quote
Old 06-17-2008, 12:04 PM   #2
C++ Witch
 
laserlight's Avatar
 
Join Date: Oct 2003
Location: Singapore
Posts: 10,353
sizeof(int) returns the number of bytes in an int. The number of chars for a numeric string representation of an int, on the other hand, could be more than sizeof(int).
__________________
C + C++ Compiler: MinGW port of GCC
Build + Version Control System: SCons + Bazaar

Look up a C/C++ Reference and learn How To Ask Questions The Smart Way
laserlight is online now   Reply With Quote
Old 06-17-2008, 12:13 PM   #3
and the hat of sweating
 
Join Date: Aug 2007
Location: Toronto, ON
Posts: 3,120
In addition to LaserLight's comment, you also need 1 more char for the NUL string terminator.

EDIT: Oops, I thought you used strlen("dark_ok__stupid"). In that case you'd need 1 more char. I think sizeof("dark_ok__stupid") should count the NUL character for you.

Last edited by cpjust; 06-17-2008 at 12:15 PM.
cpjust is offline   Reply With Quote
Old 06-17-2008, 12:42 PM   #4
Mysterious C++ User
 
Join Date: Oct 2007
Posts: 14,099
Implicit main is bad.
testident lacks a return type.
testident also takes no size argument, which makes it very vulnerable to buffer overruns.

Code:
ident_a=malloc(sizeof(the_argument));
Is also suspect, because if the_argument is a pointer, it will return the size of the pointer. I don't see where the_argument is defined.
__________________
Using: Microsoft Windows™ 7 Professional (x64), Microsoft Visual Studio™ 2008 Team System
I dedicated my life to helping others. This is only a small sample of what they said:
"Thanks Elysia. You're a programming master! How the hell do you know every thing?"
Quoted... at least once.
Quote:
Originally Posted by cpjust
If C++ is 2 steps forward from C, then I'd say Java is 1 step forward and 2 steps back.
Elysia is offline   Reply With Quote
Old 06-17-2008, 12:56 PM   #5
and the Hat of Guessing
 
tabstop's Avatar
 
Join Date: Nov 2007
Posts: 8,740
Quote:
Originally Posted by cpjust View Post

EDIT: Oops, I thought you used strlen("dark_ok__stupid"). In that case you'd need 1 more char. I think sizeof("dark_ok__stupid") should count the NUL character for you.
I think sizeof("dark_ok__stupid") would be sizeof(char *).
Edit: Never mind, I'm an idiot. (I wish I had strikethrough.)

Last edited by tabstop; 06-17-2008 at 01:09 PM.
tabstop is offline   Reply With Quote
Old 06-17-2008, 12:57 PM   #6
Mysterious C++ User
 
Join Date: Oct 2007
Posts: 14,099
In my own tests, it will give the length of the actual string, not sizeof(char*).
__________________
Using: Microsoft Windows™ 7 Professional (x64), Microsoft Visual Studio™ 2008 Team System
I dedicated my life to helping others. This is only a small sample of what they said:
"Thanks Elysia. You're a programming master! How the hell do you know every thing?"
Quoted... at least once.
Quote:
Originally Posted by cpjust
If C++ is 2 steps forward from C, then I'd say Java is 1 step forward and 2 steps back.
Elysia is offline   Reply With Quote
Old 06-17-2008, 01:01 PM   #7
C++ Witch
 
laserlight's Avatar
 
Join Date: Oct 2003
Location: Singapore
Posts: 10,353
Quote:
In my own tests, it will give the length of the actual string, not sizeof(char*).
Yes, since "dark_ok__stupid" is a const char[16], not a char*.
__________________
C + C++ Compiler: MinGW port of GCC
Build + Version Control System: SCons + Bazaar

Look up a C/C++ Reference and learn How To Ask Questions The Smart Way
laserlight is online now   Reply With Quote
Old 06-17-2008, 01:10 PM   #8
and the Hat of Guessing
 
tabstop's Avatar
 
Join Date: Nov 2007
Posts: 8,740
Yes, that's correct. I don't know what I was thinking, to be honest.
tabstop is offline   Reply With Quote
Old 06-18-2008, 04:31 PM   #9
FOSS Enthusiast
 
Join Date: Jun 2008
Location: Germany
Posts: 64
I'd rather do it like this:

Code:
char rstring[11+strlen("dark_ok__stupid")];
snprintf(rstring, sizeof(rstring), "dark_ok_%d_stupid", rand());
the 11 is the maximum number of chars, which the rand() can take (−2147483648 to 2147483647).
And actually I'd always use snprintf(), because it prevents the app from a buffer overflow if correctly used.

But don't try that with a char* because sizeof(char*) is always 8, which will lead to funny behaviour
mkruk is offline   Reply With Quote
Old 06-18-2008, 04:44 PM   #10
and the hat of sweating
 
Join Date: Aug 2007
Location: Toronto, ON
Posts: 3,120
Quote:
Originally Posted by mkruk View Post
But don't try that with a char* because sizeof(char*) is always 8, which will lead to funny behaviour
Only on 64-bit systems.
On 32-bit systems it's 4 bytes.
cpjust is offline   Reply With Quote
Old 06-18-2008, 04:45 PM   #11
Registered User
 
Join Date: Oct 2001
Posts: 2,936
>the 11 is the maximum number of chars, which the rand() can take (−2147483648 to 2147483647).
rand() is never negative.

>But don't try that with a char* because sizeof(char*) is always 8
Not always.
__________________
http://www.freechess.org
swoopy is offline   Reply With Quote
Old 06-20-2008, 04:06 AM   #12
FOSS Enthusiast
 
Join Date: Jun 2008
Location: Germany
Posts: 64
my bad, I'm working on 64Bit and forgot its 4 byte on 32Bit systems.

the return type of rand() is int, so its technically −2147483648 to 2147483647. But if it does only return positiv values, thats no big deal.. the string will just need 1 byte less because the minus will never appear
mkruk is offline   Reply With Quote
Old 06-20-2008, 04:07 AM   #13
Kernel hacker
 
Join Date: Jul 2007
Location: Farncombe, Surrey, England
Posts: 15,686
Quote:
Originally Posted by mkruk View Post
my bad, I'm working on 64Bit and forgot its 4 byte on 32Bit systems.

the return type of rand() is int, so its technically −2147483648 to 2147483647. But if it does only return positiv values, thats no big deal.. the string will just need 1 byte less because the minus will never appear
It should return values from 0 to MAX_RAND, which should be a positive value as far as I undertsand. MAX_RAND is rarely equal to MAX_INT.

--
Mats
__________________
Compilers can produce warnings - make the compiler programmers happy: Use them!
Please don't PM me for help - and no, I don't do help over instant messengers.
matsp is offline   Reply With Quote
Old 06-20-2008, 04:11 AM   #14
Mysterious C++ User
 
Join Date: Oct 2007
Posts: 14,099
According to MSDN, RAND_MAX is a signed short (32767).
Quote:
Originally Posted by MSDN
The rand function returns a pseudorandom integer in the range 0 to RAND_MAX (32767). Use the srand function to seed the pseudorandom-number generator before calling rand.
__________________
Using: Microsoft Windows™ 7 Professional (x64), Microsoft Visual Studio™ 2008 Team System
I dedicated my life to helping others. This is only a small sample of what they said:
"Thanks Elysia. You're a programming master! How the hell do you know every thing?"
Quoted... at least once.
Quote:
Originally Posted by cpjust
If C++ is 2 steps forward from C, then I'd say Java is 1 step forward and 2 steps back.
Elysia is offline   Reply With Quote
Old 06-20-2008, 04:15 AM   #15
Kernel hacker
 
Join Date: Jul 2007
Location: Farncombe, Surrey, England
Posts: 15,686
Quote:
Originally Posted by Elysia View Post
According to MSDN, RAND_MAX is a signed short (32767).
Yes, but it's implementation dependant - GNU C library has RAND_MAX as 2147483647

It is (probably) guaranteed to be at least 32767.

--
Mats
__________________
Compilers can produce warnings - make the compiler programmers happy: Use them!
Please don't PM me for help - and no, I don't do help over instant messengers.
matsp is offline   Reply With Quote
Reply

Tags
buffer, malloc, overflow, random, sprintf

Thread Tools
Display Modes

Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
Converting a circulating mouse pointer to use a Potentionmeter phoenix23 C Programming 16 10-29-2006 05:04 AM
Question on buffer overflows maxhavoc C++ Programming 3 11-25-2004 03:48 PM
buffer contents swapping daluu C++ Programming 7 10-14-2004 02:34 PM
Avoiding Buffer Overflows Aidman C++ Programming 5 01-03-2004 12:21 PM
Console Screen Buffer GaPe Windows Programming 0 02-06-2003 05:15 AM


All times are GMT -6. The time now is 11:37 PM.


Powered by vBulletin® Version 3.8.1
Copyright ©2000 - 2009, Jelsoft Enterprises Ltd.
Search Engine Optimization by vBSEO 3.3.0 RC2

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22