Bugs item #2609489, was opened at 2009-02-17 17:05
Message generated for change (Comment added) made by bagder
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2609489&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: debug/info output
Group: wrong behaviour
>Status: Closed
Resolution: Invalid
Priority: 5
Private: No
Submitted By: Garry Lancaster (gslancaster)
Assigned to: Daniel Stenberg (bagder)
Summary: vsnprintf called incorrectly in Curl_infof and elsewhere

Initial Comment:
The Curl_infof function is coded as:

---------BEGIN CODE---------
void Curl_infof(struct SessionHandle *data, const char *fmt, ...)
{
  if(data && data->set.verbose) {
    va_list ap;
    size_t len;
    char print_buffer[2048 + 1];
    va_start(ap, fmt);
    vsnprintf(print_buffer, sizeof(print_buffer), fmt, ap);
    va_end(ap);
    len = strlen(print_buffer);
    Curl_debug(data, CURLINFO_TEXT, print_buffer, len, NULL);
  }
}
---------END CODE-----------

However, vsnprintf's return value is defined as follows in the C99 standard:

"The vsnprintf function returns the number of characters that would have been written
had n been sufficiently large, not counting the terminating null character, or a neg ative
value if an encoding error occurred. Thus, the null-terminated output has been
completely written if and only if the returned value is nonnegative and less than n."

This means that, in this specific case, if print_buffer is not large enough for the vsnprintf output, then print_buffer will not be null-terminated. When strlen is called a couple of lines later, it will run past the end of print_buffer looking for the terminating null. This is a form of buffer overrun.

Similar issues exist for other calls to vsnprintf throughout the codebase.

Recommended fix: get the return value of every call to vsnprintf. Only read from buffer, vnsprintf's first parameter, if the return value is non-negative and less than n, vsnprintf's second parameter.

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

>Comment By: Daniel Stenberg (bagder)
Date: 2009-02-18 00:02

Message:
It is opaque solely for the reason that it is very close to the "real
thing" so very few developers have to consider the differences... And quite
possibly a fine future improvement would be to make it completely C99
compliant (at least for the parts we use it for). Thanks anyway for helping
out!

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

Comment By: Garry Lancaster (gslancaster)
Date: 2009-02-17 17:59

Message:
OK - I hadn't spotted that. It's rather opaque.

I agree the report is invalid.

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

Comment By: Daniel Stenberg (bagder)
Date: 2009-02-17 17:14

Message:
But... the C99 standard does not quite apply here since libcurl has its own
built-in vsnprintf() code so what matters here is how that works! I'm
convinced it doesn't fully adhere to C99!

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

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2609489&group_id=976
Received on 2009-02-18