Sprintf overflows my buffer -- why?

This is a discussion on Sprintf overflows my buffer -- why? within the C Programming forums, part of the General Programming Boards category; First, the code: Code: main(int argc, char * const argv[]) { char *ident_a=0; // Initialize ident_a as a null pointer ...

  1. #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.

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,781
    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
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    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.

  4. #4
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,635
    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.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  5. #5
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    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.

  6. #6
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,635
    In my own tests, it will give the length of the actual string, not sizeof(char*).
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  7. #7
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,781
    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
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  8. #8
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Yes, that's correct. I don't know what I was thinking, to be honest.

  9. #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

  10. #10
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    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.

  11. #11
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    >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.

  12. #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

  13. #13
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    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.

  14. #14
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,635
    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.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  15. #15
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    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.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 16
    Last Post: 10-29-2006, 04:04 AM
  2. Question on buffer overflows
    By maxhavoc in forum C++ Programming
    Replies: 3
    Last Post: 11-25-2004, 02:48 PM
  3. buffer contents swapping
    By daluu in forum C++ Programming
    Replies: 7
    Last Post: 10-14-2004, 02:34 PM
  4. Avoiding Buffer Overflows
    By Aidman in forum C++ Programming
    Replies: 5
    Last Post: 01-03-2004, 11:21 AM
  5. Console Screen Buffer
    By GaPe in forum Windows Programming
    Replies: 0
    Last Post: 02-06-2003, 04:15 AM

Tags for this Thread


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