Sprintf overflows my buffer -- why?

**laserlight** · 06-20-2008, 05:47 AM

Quote:

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

Definitely.

Lasston · 06-20-2008, 01:42 PM

Sorry, but this doesn't seem to be working universally...

I managed to get that specific spot of code working with mkruk's solution (not making it a pointer) but it isn't working later in the code, specifically:

(ident_a is a const char* that has already been successfully allocated and initialized here-- in my tests i'm setting it to abcdefghijklmnopqrstuvwxyz0123456789 because it's a good test string)

Code:

char call[sizeof(ident_a) + strlen("security lock-keychain .keychain >& /dev/null")];
snprintf(call, sizeof(call), "security lock-keychain %s.keychain >& /dev/null", keychain);
int results=system(call);

The point is to make a system call with ident_a in the middle of it... but when I do the snprintf, I end up with the string "security lock-keychain abcdefghijklmnopqrstuvwxy" in call. I just used mkruk's solution again, why doesn't it work this time?

Elysia · 06-20-2008, 02:03 PM

char call[sizeof(ident_a) + strlen("security lock-keychain .keychain >& /dev/null")];

Illegal. Size is not constant.
However, you could do:

char call[sizeof(ident_a) + sizeof("security lock-keychain .keychain >& /dev/null")];

The problem is that the buffer is not large enough to hold your entire string. It is only large enough to hold "security lock-keychain %s.keychain >& /dev/null" minus the "%s".

Lasston · 06-20-2008, 02:16 PM

OK two quickie questions about this:

1) If mkruk's idea is illegal, why does it work perfectly earlier in the code? I use

Code:

char rstring[11+strlen("dark__fun")];
snprintf(rstring, sizeof(rstring), "dark_%d_fun", (rand()*rand()));

with no problems whatsoever.

2) I replaced strlen with sizeof and I'm still not getting the full string printed, it's now giving out after "z"... is there any way I can do what I'm trying to?

Elysia · 06-20-2008, 02:18 PM

Quote:

Originally Posted by Lasston

1) If mkruk's idea is illegal, why does it work perfectly earlier in the code? I use

Code:

char rstring[11+strlen("dark__fun")];
snprintf(rstring, sizeof(rstring), "dark_%d_fun", (rand()*rand()));

with no problems whatsoever.

It's true that it compiles, but that's because it's a compiler extension. In the standard, it's illegal.

**laserlight** · 06-20-2008, 02:19 PM

Quote:

1) If mkruk's idea is illegal, why does it work perfectly earlier in the code?

Well... it is legal in C99 as it uses the variable length array feature. I am not sure if you really want to use that though (and honestly I am not familiar with it myself).

Lasston · 06-20-2008, 02:37 PM

I am trying to learn standard, unextended C right now, so I guess I'll be taking out the array.

So... is there just no way to do something like this in the C language? It seems like no matter what I do I get an overflow.

**laserlight** · 06-20-2008, 02:43 PM

Quote:

I am trying to learn standard, unextended C right now, so I guess I'll be taking out the array.

C99 is the 1999 edition of the C Standard. It so happens that the variable length array feature is a C++ extension for the g++ compiler, but that is not relevant here.

Quote:

So... is there just no way to do something like this in the C language? It seems like no matter what I do I get an overflow.

You can use dynamic memory allocation with malloc() and free() from <stdlib.h>.

matsp · 06-20-2008, 03:00 PM

Quote:

Originally Posted by Lasston

So... is there just no way to do something like this in the C language? It seems like no matter what I do I get an overflow.

Aside from Laserlights suggestion, there is always the option of "oversize" - if you KNOW that your sprintf() will not produce more than a few hundred bytes, make your string 4096 or something "ridiculuously large".

Naturally, this doesn't work if you have completely and utterly arbitrary inputs - in that case, you will have to find a way to calculate how much space it takes and then use malloc/free.

--
Mats

swoopy · 06-20-2008, 03:06 PM

>1) If mkruk's idea is illegal, why does it work perfectly earlier in the code?

Code:

>char call[sizeof(ident_a) + strlen("security lock-keychain .keychain >& /dev/null")];
>snprintf(call, sizeof(call), "security lock-keychain %s.keychain >& /dev/null", keychain);
>int results=system(call);

Make this:

Code:

char call[1 + strlen(keychain) + strlen("security lock-keychain .keychain >& /dev/null")];
snprintf(call, sizeof(call), "security lock-keychain %s.keychain >& /dev/null", keychain);
int results=system(call);

dwks · 06-20-2008, 03:58 PM

That's still C99.

strlen() is a function. Its return value can only be evaluated at runtime. Consequentally, the compiler doesn't know how big the array is going to be, and you're using variable length arrays.

To solve this problem, the first thing you need to do is figure out how many digits rand() can return. One way to do that is to look at RAND_MAX and count its digits.

Code:

int digits_in_number(int n) {
    int digits = 0;

    while(n) {
        digits ++;
        n /= 10;
    }

    return digits;
}

digits_in_number(RAND_MAX);

Of course, you might be even better off passing the actual random number to a function like that, to get the exact amount of memory that you need.

Note that I've created a function to figure out how much memory to allocate, so again, you'll either have to use VLAs (variable length arrays) or dynamic memory allocation. I suggest memory allocation. It's not too hard.

Code:

char *call = malloc(digits_in_number(ident_a)
    + strlen("security lock-keychain .keychain >& /dev/null") + 1);  /* +1 for the NULL */
int result;
sprintf(call, "security lock-keychain %s.keychain >& /dev/null", keychain);
result = system(call);
free(call);

A few notes about my version:

I'm using sprintf() here, instead of snprintf(). The latter is C99, so if you're using it you might as well use variable length arrays anyway.
I'm declaring result at the beginning of a block (right after an opening curly brace), instead of the middle of the block. The latter, as you had it, is also C99.
I'm adding one to the length of the string, to accomodate the NULL terminator.

I'm not sure why you're adding the size of ident_a to the string, though, because you're not using ident_a anywhere. Perhaps it should be passed to the sprintf() someplace?

Anyway . . . hopefully that helped a little bit . . . .

swoopy · 06-20-2008, 04:33 PM

>I'm not sure why you're adding the size of ident_a to the string, though
That's supposed to be keychain. That's the only change I made from his original code, plus adding one for the string terminator.

06-20-2008, 01:42 PM	#17
Lasston Registered User Join Date: Jun 2008 Posts: 4	Sorry, but this doesn't seem to be working universally... I managed to get that specific spot of code working with mkruk's solution (not making it a pointer) but it isn't working later in the code, specifically: (ident_a is a const char* that has already been successfully allocated and initialized here-- in my tests i'm setting it to abcdefghijklmnopqrstuvwxyz0123456789 because it's a good test string) Code: char call[sizeof(ident_a) + strlen("security lock-keychain .keychain >& /dev/null")]; snprintf(call, sizeof(call), "security lock-keychain %s.keychain >& /dev/null", keychain); int results=system(call); The point is to make a system call with ident_a in the middle of it... but when I do the snprintf, I end up with the string "security lock-keychain abcdefghijklmnopqrstuvwxy" in call. I just used mkruk's solution again, why doesn't it work this time?
Lasston is offline

06-20-2008, 02:16 PM	#19
Lasston Registered User Join Date: Jun 2008 Posts: 4	OK two quickie questions about this: 1) If mkruk's idea is illegal, why does it work perfectly earlier in the code? I use Code: char rstring[11+strlen("dark__fun")]; snprintf(rstring, sizeof(rstring), "dark_%d_fun", (rand()*rand())); with no problems whatsoever. 2) I replaced strlen with sizeof and I'm still not getting the full string printed, it's now giving out after "z"... is there any way I can do what I'm trying to?
Lasston is offline

06-20-2008, 02:37 PM	#22
Lasston Registered User Join Date: Jun 2008 Posts: 4	I am trying to learn standard, unextended C right now, so I guess I'll be taking out the array. So... is there just no way to do something like this in the C language? It seems like no matter what I do I get an overflow.
Lasston is offline

06-20-2008, 03:58 PM	#26
dwks Frequently Quite Prolix Join Date: Apr 2005 Location: Canada Posts: 7,629	That's still C99. strlen() is a function. Its return value can only be evaluated at runtime. Consequentally, the compiler doesn't know how big the array is going to be, and you're using variable length arrays. To solve this problem, the first thing you need to do is figure out how many digits rand() can return. One way to do that is to look at RAND_MAX and count its digits. Code: int digits_in_number(int n) { int digits = 0; while(n) { digits ++; n /= 10; } return digits; } digits_in_number(RAND_MAX); Of course, you might be even better off passing the actual random number to a function like that, to get the exact amount of memory that you need. Note that I've created a function to figure out how much memory to allocate, so again, you'll either have to use VLAs (variable length arrays) or dynamic memory allocation. I suggest memory allocation. It's not too hard. Code: char call = malloc(digits_in_number(ident_a) + strlen("security lock-keychain .keychain >& /dev/null") + 1); / +1 for the NULL / int result; sprintf(call, "security lock-keychain %s.keychain >& /dev/null", keychain); result = system(call); free(call); A few notes about my version: I'm using sprintf() here, instead of snprintf(). The latter is C99, so if you're using it you might as well use variable length arrays anyway. I'm declaring result at the beginning of a block (right after an opening curly brace), instead of the middle of the block. The latter, as you had it, is also C99. I'm adding one to the length of the string, to accomodate the NULL terminator. I'm not sure why you're adding the size of ident_a to the string, though, because you're not using ident_a anywhere. Perhaps it should be passed to the sprintf() someplace? Anyway . . . hopefully that helped a little bit . . . . __________________ dwk Seek and ye shall find. quaere et invenies.* "Simplicity does not precede complexity, but follows it." -- Alan Perlis "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra "The only real mistake is the one from which we learn nothing." -- John Powell Other boards: DaniWeb, TPS Unofficial Wiki FAQ: cpwiki.sf.net My website: http://dwks.theprogrammingsite.com/ Projects: codeform, xuni, atlantis, etc. New project: nort
dwks is offline

06-20-2008, 04:33 PM	#27
swoopy Registered User Join Date: Oct 2001 Posts: 2,936	>I'm not sure why you're adding the size of ident_a to the string, though That's supposed to be keychain. That's the only change I made from his original code, plus adding one for the string terminator. __________________ http://www.freechess.org
swoopy is offline

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

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