Wireshark · Wireshark-dev: [Wireshark-dev] how to handle dissecting length encoded strings

[Fulko Hew <fulko.hew@xxxxxxxxx>]

I'm looking into a bug in a dissector, and I don't see what the 'right
way' to fix it would be.
Here's the existing code snippet:

static int
dissect_octet_string(tvbuff_t *tvb, proto_tree *tree, int offset, char flags)
{
    guint32 n_oct, p_noct;
    char context[1024];

    NORLEL(flags, n_oct, tvb, offset);

    p_noct = PADDING(n_oct);
    if (n_oct >= 1024)
        THROW(ReportedBoundsError);
    if (n_oct > 0)
        count = tvb_get_nstringz(tvb, offset + 4, n_oct, context);
    context[n_oct]='\0';

    proto_tree_add_uint(tree, hf_ostring_len, tvb, offset, 4, n_oct);
    proto_tree_add_string(tree, hf_ostring, tvb, offset + 4, n_oct, context);
    return p_noct + 4;
}

The important part is that this code figures out how long the length
encoded string is, and attempts to copy that much of the string
via tvb_get_nstringz() into the buffer called 'context'.
The trouble is that the source string isn't null terminated, and when
the string is one byte long... this code fails.

My thoughts are this code is better served by using tvb_get_ephemeral_string()
[but I'm not sure enough, I'm looking for validation from someone]
I'd propose replacing the above routine with something like:

dissect_octet_string(tvbuff_t *tvb, proto_tree *tree, int offset, char flags)
{
    guint32 n_oct, p_noct;
    guint8 *str_buf

    NORLEL(flags, n_oct, tvb, offset);

    p_noct = PADDING(n_oct);
    if (n_oct >= 1024)
        THROW(ReportedBoundsError);
    tvb_get_ephemeral_string(tvb, offset +4, n_oct);

    proto_tree_add_uint(tree, hf_ostring_len, tvb, offset, 4, n_oct);
    proto_tree_add_string(tree, hf_ostring, tvb, offset + 4, n_oct, str_buf);
    return p_noct + 4;
}

Comments anyone?
Fulko