On Tue, 2005-08-23 at 16:46 -0400, Charles Schmidt wrote:
> All set as far as this goes, I think.
I think we still need to check that the codesize matches what we expect
for each type. For example, say you have this as the sole contents of
the buffer:
<content code:INT64><codesize:1><1 byte>
Note this input contains an invalid codesize; it should be 8. At least,
that's what I'm assuming the DAAP protocol says, maybe the codesize is
only relevant for _TYPE_STRING, but the present code does rely on it.
So given this input to rb_daap_structure_parse_container_buffer, the
variables initially will be:
l = 0
buf_length = 9
This will pass the if (codesize > buf_length - l - 4 || codesize < 0)
check. Then going down, we drop to the case RB_DAAP_TYPE_INT64,
and we have l = 8. But rb_daap_buffer_read_int64 is going to read 8
bytes, and we only have 1 left, so we'll overread the buffer.
My original suggestion for one of your earlier patches was just to check
the buffer size remaining before we do any reading. I think it will be
equivalent to validate the codesize; this scenario relies on the input
containing an invalid codesize. After reading through So in your
current code, you'd change it to look like this:
case RB_DAAP_TYPE_INT64: {
gint64 val;
if (codesize != 8)
val = 0;
else
val = rb_daap_buffer_read_int64(&(buf[l]));
...
}
Does that make sense?
And similarly for the _TYPE_VERSION, _TYPE_INT, etc. cases.
Oh...one other thing I just noticed. Many parts of the Rhythmbox code
assume that strings are UTF-8. We should add a call to g_utf8_validate
for the RB_DAAP_TYPE_STRING case.
Attachment:
signature.asc
Description: This is a digitally signed message part