Solved

Sprintf() causing core dump sporadically on old existing code.

Posted on 2003-11-14
13
1,565 Views
Last Modified: 2013-12-14
Sun Solaris 2.8 running in 32 bit mode with 6.2 compiler.
I've been running into a problem sporadically in the use of sprintf:

sprintf(char_string, " ... %s", function());

where function() returns a character string

This has been causing core dumps in some places (function return is probably in limbo), often in old code that has been in production for years.  It has been popping up when I make changes, usually completely unrelated, other places in the code.  So far it has always been caught in development, but may become a production issue.

***WHY***
To fix this copy the function return to a character array and pass that in to sprintf.

This has been a rare bug so far, but you may more of a problem and I need to come up with the cause. Any ideas?
0
Comment
Question by:rayskelton
  • 4
  • 3
  • 2
  • +3
13 Comments
 
LVL 16

Assisted Solution

by:_nn_
_nn_ earned 250 total points
Comment Utility
>> where function() returns a character string

Depends on where that returned string was allocated. If it's a simple variable local to function() (a char[]) then it's just invalid. If it's a static buffer, then you could investigate multi-threading issues. More informations about that function wouldn't hurt...

Finally, sprintf() is dangerous per se, how do you guaranty that char_string is large enough ? Better use snprintf (if supported, I don't know Solaris compilers)
0
 

Author Comment

by:rayskelton
Comment Utility
The char_string in sprintf(char_string, " ... %s", function()); is always allocated and has been running for years, but just recently became an issue. If you allocate a seperate buffer to hold the result of sprintf(), then copy this buffer into the destination string, everyone is happy.

0
 
LVL 16

Expert Comment

by:_nn_
Comment Utility
Ok, if you don't mind, let us put the issue about sprintf itself by the side. I disagree with you about that C standard function and specially about things that have been "running for years", but we'll come back to it after we've checked what looks more important to me.

Generally speaking, the amount of informations and source code you're revealing is pretty tiny, specially when trying to resolve a "sporadic crash". When I said "More informations about that function wouldn't hurt...", I was talking about the called some_function in

sprintf(char_string, " ... %s", some_function());

Can you please give some more details about some_function() ? What exactly is returned and how ? Again, is that application multi-threaded or not ? When you say :

>> To fix this copy the function return to a character array and pass that in to sprintf.

... it suggests that something wrong is going on within that function. If possible, paste the source code here.

Thanks.
0
 
LVL 45

Assisted Solution

by:Kdo
Kdo earned 200 total points
Comment Utility

Hi rayskelton,

You can definitively prove (or disprove) if the sprintf() is the cause of the failure or if something else is clobberring your program and it only shows up at this point.

Redefine char_string as dynamic and make a new variable for holding the value returned by function().  Also create a static variable with your format string.

char *char_string = NULL;
char *function_value;
char *FormatString = ".... %s";

Then change the sprintf() to look like this:

function_value = function ();
char_string = realloc (char_string, strlen(FormatString) + strlen (function_value));
sprintf (char_string, FormatString, function_value);


If the code works, then the string generated by sprintf() was longer than was allocated for char_string.  If the code still fails, the problem lies elsewhere in your program.

I assume that your format string is more complicated that what you indicated so make sure that you realloc() a buffer large enough for the string.  The length generated in my example is sufficiently large for this example because the "%s" substring is counted for determining the length, but will be dropped when the substitution is made -- hence the buffer is long enough for the data and the terminating zero.


Good Luck!
Kent
0
 
LVL 11

Expert Comment

by:dimitry
Comment Utility
It may be important to check _nn_ suggestion. Can you please show use function() code ?
Not the whole code but the part where is return value is allocated...
0
 
LVL 45

Expert Comment

by:Kdo
Comment Utility

Hi Ray,

One more thing to check...  Does function() ALWAYS return a non-null value?  There is no guarantee when dereferencing NULL so you could wind up with an error that results in a core dump.  From my previous example, add 1 line so the call to function() is now these two lines:

function_value = function ();
function_value = function_value ? function_value : "<function returned null>";


Again, Good Luck,
Kent
0
Why You Should Analyze Threat Actor TTPs

After years of analyzing threat actor behavior, it’s become clear that at any given time there are specific tactics, techniques, and procedures (TTPs) that are particularly prevalent. By analyzing and understanding these TTPs, you can dramatically enhance your security program.

 
LVL 3

Expert Comment

by:guynumber5764
Comment Utility
The two main things that'll cause sprintf(sdst, "%s", ssrc) to crap out are 1) ssrc==NULL and 2) strlen(ssrc) > sizeof(sdst) (usually because ssrc was not terminated).
0
 
LVL 22

Accepted Solution

by:
grg99 earned 50 total points
Comment Utility

It's always risky to use sprintf() with a %s format string.... How do you know the string is going to fit in the destination char array?.   Much better to use snprintf().  Also it wouldnt hurt to add some checks that the function() always returns a proper char * that isnt NULL or garbage.  And the function() better not return a pointer to a char array local to function(), as that no longer exists by the time sprintf() gets called.

0
 

Author Comment

by:rayskelton
Comment Utility
Therse are many good suggestions. I will try some of these today and try to get some code samples posted. Thanks to all for suggestions.
0
 

Author Comment

by:rayskelton
Comment Utility
The offending code is listed below. The snprintf() resolves all related core dumps, so I am happy with that solution. The List class is around 12 years old and used in thousands of places through the company, so I want to avoid this if possible. This is the method returned in the sprintf(). This just began around siz months ago and is around the same time we converted this code to 64 bit. We have had several bad behaving occurrences of old code, since the upgrade. ll->data is a node within a linked list.

int kb215_add_reason_codes(List & msg_list, NcfResult & ncf)
{
  List *ll = (List *) &ncf.pls_mssg();
  for(ll->rewind(); ll->data() ; (*ll)++ )
  {
    char temp[80];
    sprintf(temp, (char *)"  %s", (char *)ll->data());
    msg_list.append(ll_strdup(temp));
  }

  return(ERR_OK);
}

0
 
LVL 22

Expert Comment

by:grg99
Comment Utility
The sprintf is being used here as a slow and disaster-prone way to put a space in front of a string.

This is a bit faster and safer:

char temp[ 80 ];

strncpy( temp, "  ", sizeof( temp ) );
strncat( temp, ll->data(), sizeof( temp ) );

-------------

You might also try adding a few if() statements to catch the array overrun condition and log it somewhere.
This little problem may be a symptom of a larger problem somewhere else.  Maybe somehow bad data is getting
into your lists.  It would be nice to track down the source of the problem instead of just patching this hole.



0
 
LVL 45

Expert Comment

by:Kdo
Comment Utility

Hi Ray,

sprintf() may have been the quick and easy way to put a space in front of a string (message), but it sure did cause a headache, huh?  If you malloc() a buffer that's large enough for the new string you should be just fine...

Congratulations on isolating the problem!
Kent


int kb215_add_reason_codes(List & msg_list, NcfResult & ncf)
{
  char *temp = NULL;
  List *ll = (List *) &ncf.pls_mssg();
  for(ll->rewind(); ll->data() ; (*ll)++ )
  {
    temp = (char *) realloc (temp, strlen (ll->data)+1+1);  /*  Add one for the leading space, and one more for the zero terminator  */
    strcpy (temp, " ");
    strcat (temp, (char *)ll->data());
    msg_list.append(ll_strdup(temp));
  }
  free (temp);
  return(ERR_OK);
}
0
 

Author Comment

by:rayskelton
Comment Utility
I agree the Linked list code needs some serious scrutiny and is high on my list.

 Thanks to all.
0

Featured Post

Maximize Your Threat Intelligence Reporting

Reporting is one of the most important and least talked about aspects of a world-class threat intelligence program. Here’s how to do it right.

Join & Write a Comment

Summary: This tutorial covers some basics of pointer, pointer arithmetic and function pointer. What is a pointer: A pointer is a variable which holds an address. This address might be address of another variable/address of devices/address of fu…
This is a short and sweet, but (hopefully) to the point article. There seems to be some fundamental misunderstanding about the function prototype for the "main" function in C and C++, more specifically what type this function should return. I see so…
Video by: Grant
The goal of this video is to provide viewers with basic examples to understand and use nested-loops in the C programming language.
This tutorial covers a step-by-step guide to install VisualVM launcher in eclipse.

743 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

18 Experts available now in Live!

Get 1:1 Help Now