Ethereal-dev: RE: [Ethereal-dev] Weird bug on MSVC build only (probably HTTP or tvb_get_ptr)
|From: Guy Harris
|
|Biot Olivier said:
|> If I inspect the last function from the stack, the debugger points at
|> basic_response_dissector() in packet-http.c at the if statement:
|>
|> if (sscanf((const gchar *)data, "%d.%d %d", &minor, &major,
|> &status_code) == 3) {
|
|Oh, *that's* not good. As far as I know, "sscanf()" takes, as
|its first
|argument, a C string - i.e., it has to be null-terminated, and...
|
|> If I inspect the value returned from the statement on the line before
|> the if statement:
|>
|> data = tvb_get_ptr(tvb, 5, 12);
|
|...that's not guaranteed to be null-terminated. (I'm not sure it's
|guaranteed to have 12 characters, either....)
I feared this.
By replacing the tvb_get_ptr() call with a g_strndup() of the desired part
of the tvb_get_ptr() data the crash was eliminated.
I'd like to know what tvb_get_ptr() was intended to do and what it is really
doing, as it looks like both stories don't match :)
AFAIK tvb_get_ptr() was intended to return a pointer to contiguous data of
the specified length (maybe even after reassembly). If this data does not
exist, an appropriate exception is thrown. How it is implemented is another
story (I didn't look at the code right now).
|Perhaps it should, instead, parse the response line itself, using
|"req_strlen" as an indication of the number of characters in the string
|*after* the first 5 characters rather than expecting "data" to be
|null-terminated.
I currently wrote the following fix (packet-http.c):
static void
basic_response_dissector(tvbuff_t *tvb, proto_tree *tree, int resp_strlen)
{
gchar *data;
int minor, major, status_code;
data = g_strndup((const gchar *)tvb_get_ptr(tvb, 5, resp_strlen),
resp_strlen);
if (sscanf((const gchar *)data, "%d.%d %d", &minor, &major,
&status_code) == 3) {
proto_tree_add_uint(tree, hf_http_response_code, tvb, 9, 3,
status_code);
stat_info->response_code = status_code;
}
g_free(data);
}
Should we provide a tvb_sscanf() function for implementing such
functionality? If we have some open-source sscanf() code we can use (from
GCC libc maybe), this could be done "easily", I guess.
Regards,
Olivier
